This is more of a RuboCop improvement, consider offering PR upstream.

The issue is that some files analysed use tabs - which then makes the error markers not properly line up. (Using the default reporter to console.)

I don't understand what this sentence is trying to say:

Running only cops for Extension Warehouse Technical requirements

If you want to focus on only the technical requirements for having your extension hosted on Extension Warehouse then you can limit the cop to only the SketchupRequirements department. This is the most important department which you should ignore.

It appear that RuboCop isn't handling END well. Some extensions are using
this to include comments or dead code after this.

Related GitHub issues:

• rubocop/rubocop#1541 (Resolved)
• rubocop/rubocop@7fcee5b

This might be due to the progress reporter. If we run just plain json report
we might be fine.

Can some basic inspection be made that checks if entity parameters from observers are being checked by .deleted? or .valid? before being used?

Due to how require and Sketchup.require checks whether a file has been loaded, the following files should always be required in the exact given manner:

require 'sketchup.rb'
require 'extensions.rb'
require 'langhandler.rb'

Any other variants, such as omitting the file extension or capitalization might cause the file to double load.

Add a cop to check for use of namespaces for shipped extensions. (Might have to look into historic changes.)

rubocop/rubocop#5656 (comment)

I keep running into errors with node matcher.

The latest case is a snippet like this:

module Example
  msg = "Hello World"
  msg += "Foo Bar"
end

A matcher like this:

        def_node_search :sketchup_extension_new, <<-PATTERN
          ({:lvasgn :ivasgn :cvasgn :gvasgn :casgn} ...
            (:send
              (:const nil? :SketchupExtension) :new
              _
              _))
        PATTERN

Inspection like this:

def investigate(processed_source)
  source_node = processed_source.ast
  extension_nodes = sketchup_extension_new(source_node).to_a
  # ...
end

Which leads to error like this:

  1) RuboCop::Cop::SketchupRequirements::SketchupExtension Default source path does not throw an error when inspecting source
     Failure/Error: extension_nodes = sketchup_extension_new(source_node).to_a

     NoMethodError:
       undefined method `type' for :msg:Symbol
     # C:3:in `block in sketchup_extension_new'
     # C:2:in `sketchup_extension_new'
     # ./lib/rubocop/sketchup/requirements/sketchup_extension.rb:44:in `each'
     # ./lib/rubocop/sketchup/requirements/sketchup_extension.rb:44:in `to_a'
     # ./lib/rubocop/sketchup/requirements/sketchup_extension.rb:44:in `investigate'
     # ./spec/rubocop/sketchup/requirements/sketchup_extension_spec.rb:93:in `block (3 levels) in <top (required)>'

My spec to reproduce:

    it 'does not throw an error when inspecting source' do
      inspect_source(['module Example',
                      '  msg = "Hello World"',
                      '  msg += "Foo Bar"',
                      'end'],
                      './src/hello.rb')
      expect(cop.offenses.size).to eq(1)
    end

Any ideas? (I get lost when I try to debug into the matchers.)

Add a cop that checks for expected file structure.

Challenge here is that it's unclear how to trigger RuboCop for missing files.

ReadMe states:
'This is not intended to be used inside of SketchUp'

Do you think it would be helpful if it could? Just wondering...

Ruby under Windows doesn't provide the correct encoding for these trings.
You may experience encoding errors if you try to mix these string with other
strings. To work around this use .force_encoding('UTF-8')

This appear to fail inside RuboCop itself.

bundle exec rake spec SPEC=spec/rubocop/sketchup/suggestions/file_encoding_spec.rb:62

SketchUp will automatically resolve a file's extension to .rb, .rbs or .rbe.

Omitting file extensions is required when encrypting extensions. By always omitting the extension one avoid accidentally running into this issue.

Except 'sketchup.rb' - the load guard is case sensitive, if required with
different capitalization the file will load multiple times and add multiple
menus.

Often people hard code file extension into SketchupExtension.new.
Catch and warn against this.

We can use YARD to provide lists of new features in each SketchUp version.
Developers can then declare minimum compatibilty and have RuboCop validate
against it. Similar to how RuboCop already can check Ruby compatibilty.

yard --use-cache --list --query "object.tag(:version).text =~ /2018/"

yard --use-cache --list --query "object.tag(:version).text == 'SketchUp 6'"
yard --use-cache --list --query "object.tag(:version).text == 'SketchUp 7'"
yard --use-cache --list --query "object.tag(:version).text == 'SketchUp 8'"

yardoc -t versions -f text

LayOut 2018
LayOut 2019
SketchUp 2013
SketchUp 2014
SketchUp 2015
SketchUp 2016
SketchUp 2016 M1
SketchUp 2017
SketchUp 2018
SketchUp 2019
SketchUp 6.0
SketchUp 7.0
SketchUp 7.0 M1
SketchUp 7.1
SketchUp 7.1 M1
SketchUp 8.0
SketchUp 8.0 M1
SketchUp 8.0 M2

ukoloff/appveyor-worker#1

SketchupRequirements/RequireToolsRubyFiles isn't needed. Only older SU versions had issues here.

No need to flag or make restrictions here.

Right now multiple cops have a configuration option to specify where the source is.

See if this can be a global setting.

Identify global variables defined by the SketchUp API and add them to the list of variables to ignore in GlobalVariables.

tt_cleanup/vendor/error-handler/error_reporter.rb:225:27: E: SketchupRequirements/GlobalVariables: Do not introduce global variables.
      $LOADED_FEATURES || $loaded_files
                          ^^^^^^^^^^^^^

4 files inspected, 1 offense detected

Only transform first letter. Leave the rest alone.

This will allow strings like: "HTML" or "SketchUp" from being flagged.

See how rubocop-cask have done it. They appear to have a default yml configured with cop descriptions etc. Though they had to monkey patch rubocop by the looks of it.

This will report a violation in SketchupRequirements/GlobalMethods

module Example
  class Object
    def namespaced_example; end
  end
end

It could be helpful to developers to get a warning/error if their extensions rely on methods defined in shipped extensions. I have myself accidentally used some of these methods without realizing they weren't a part of the API and later got errors when running the extension on a computer where these extensions were disabled.

Should we check for this as well?

• $LOADED_FEATURES
• $loaded_files

This cop cannot catch everything, but obvious calls to model changing methods can be checked that they are wrapped in transparent operations.

yard --use-cache --list --query "object.path =~ /^Sketchup::Entities#add_/"

Look if build tool from rubocop-rspec can be re-purposed to automatically generate.

Use YARD to identify deprecated features. (Might be worth doing a plain text search as well.)
Add a cop for each item.

 yard --use-cache --list --query 'object.has_tag?(:deprecated)'

This also works:

 yard --use-cache --list --query '@deprecated'

There are performance issue with the OpenSSL library that Ruby ship. In a clean SU session, default model there is a small delay observed in the Windows version of SU.

But with a larger model loaded, or session that have had larger files loaded the lag will be minutes.

SecureRandom is affected by this, as it uses OpenSSL to seed.

This issue seem to have been addressed in OpenSSL 1.1 which was updated in Ruby 2.4.

openssl/openssl#1079

New version released. Best to try to keep up to date with RuboCop.

Create a reporter that compose an easy to digest overview of rubocop-sketchup violations.
HTML format.

This CL breaks our custom cops: rubocop/rubocop@a8e26ea

We need to perform matching refactoring to our use of on_method_def to account for the recent changes.

It appear that RuboCop doesn't support Exclude for departments. An issue is opened in RuboCop's tracker; rubocop/rubocop#5678

If such a feature is not desired for upstream RuboCop then alternate configuration option will be needed for this extension. Could go the same route as the RuboCop RSpec extension; https://github.com/rubocop-rspec/rubocop-rspec/blob/e51fc4ab47bc4c4a545ee94fec3f4fdfe8e6c05f/lib/rubocop/cop/rspec/cop.rb#L26-L40

It's often seen that extensions use the global variable for their own LanguageHandler. We should catch and warn about that.

Review the existing matchers that look for certain variables (mod, model etc) and make sure that instance and class variable variants are also accounted for.

The root rb file should register a SketchupExtension instance.

Need to figure out how to check for this in a particular file.

In rare cases third party libs will cause multiple root namespace. Add a config to allow to reduce noise when running the cop without having to disable the check completely.

Since there are so many extensions tapping into DCs it might be useful to reduce noise by omitting it from the SketchupRequirement department checking for global usage. At least reading, but trying to assign anything to it should still flag an offense.

There is still a cop that warns about using DC internals (not part of Requirement)

Detect use of model.selection and .add/.remove/.toggle inside
of loops. Recommend bulk actions.

Not sure if this is a bug - but need to figure out a way to catch and handle this scenario when using the gem for EW automated moderation.

rubocop/rubocop#1289

Analysing lss_archlite_1_0_5 (1800/3772) ...
invalid codepoint 0xD83D in UTF-8
C:/Ruby23/lib/ruby/gems/2.3.0/gems/parser-2.4.0.0/lib/parser/lexer.rb:16909:in `chr'
C:/Ruby23/lib/ruby/gems/2.3.0/gems/parser-2.4.0.0/lib/parser/lexer.rb:16909:in `advance'
C:/Ruby23/lib/ruby/gems/2.3.0/gems/parser-2.4.0.0/lib/parser/base.rb:231:in `next_token'
C:/Ruby23/lib/ruby/2.3.0/racc/parser.rb:259:in `_racc_do_parse_c'
C:/Ruby23/lib/ruby/2.3.0/racc/parser.rb:259:in `do_parse'
C:/Ruby23/lib/ruby/gems/2.3.0/gems/parser-2.4.0.0/lib/parser/base.rb:162:in `parse'
C:/Ruby23/lib/ruby/gems/2.3.0/gems/parser-2.4.0.0/lib/parser/base.rb:209:in `tokenize'

Detect use of include in global namespace.

require 'sketchup.rb'
include Math
module Example
  # ...
end

Not sure why this is an argument, extensions should default to just load. Otherwise it's confusing for the user.

Some extensions tries to use exit to break from methods. This is actually a call to terminate the interpreter. Currently we catch and prevent this, but not with exit!.

We should have a cop that detect usage of both exit and exit!.

when running bundle exec rake:

C:/Users/tthomas2/SourceTree/rubocop-sketchup/spec/rubocop/sketchup/requirements/ruby_core_namespace_spec.rb:18: warning: constant ::Bignum is deprecated
C:/Users/tthomas2/SourceTree/rubocop-sketchup/spec/rubocop/sketchup/requirements/ruby_core_namespace_spec.rb:18: warning: constant ::Fixnum is deprecated

Can be ignored for now - need to check for Fixnum and Bignum due to older Ruby versions used in SketchUp.

But in future Ruby versions this might break - which could cause problems when running RuboCop using a newer Ruby version.

DC defines a set of globals which several extensions are tapping into for various reasons. Some are to extend features, some are to work around bugs.

The policy on doing is is that it's entirely unsupported and on their own risk. When encountered we will review on a case by case basis.

There should be cops that check for usage of DC global variables and warn about using them.

Might want to add a cop to check for modifications of DC variables - if they are added to the GlobalVariable Cop's exclude list. There have been observations of people using the DC variables for their own extensions. (Copy+Paste)

See if we can craft a cop that checks the root rb file for includes that relate to the extension subfolder.

An extension should not load logic prior to being loaded by the SketchupExtension handler - this to ensure the user can disable the extension.

Sketchup::require 'sketchup.rb'

SketchupSuggestions/SketchupRequire: Don't hard code file extensions with Sketchup.require

Either recommend require is used to load the files from the Tools folder, or ignore these require for the SketchupSuggestions/SketchupRequire cop.

https://github.com/backus/rubocop-rspec/blob/c730e3fda4461d9297030225fcc71980b129d96a/bin/build_config

https://github.com/backus/rubocop-rspec/blob/9095beb60909b07b58adef8338c115173f295d04/.github/CONTRIBUTING.md

Some users might manage extensions manually or via third party extension
managers. In these cases using Sketchup.find_support_file will fail to
resolve the path to the files. Prefer using __FILE__ or __dir__ to resolve
files relative to the current file.