Detect top-level constant usage like ::Const

[biinari]

[Fix #8215]

Detect top-level constant usage (in offenses or allowed forms) like ::Const in the following cops:

• Gemspec/RubyVersionGlobalsUsage - ::RUBY_VERSION
• Lint/NestedMethodDefinition - ::Class.new, ::Module.new, ::Struct.new
• Lint/NonDeterministicRequireOrder - ::Dir
• Lint/RandOne - ::Kernel.rand(1)
• Lint/RedundantSplatExpansion - ::Array.new
• Lint/UnusedMethodArgument - ::NotImplementedError
• Lint/UselessAccessModifier - ::Class.new, ::Module.new, ::Struct.new
• Metrics/ClassLength - ::Class.new
• Metrics/ModuleLength - ::Module.new
• Style/DateTime - Allow using ::Date constants for historic date
• Style/Dir - ::File
• Style/EmptyLiteral - ::Array.new, ::Hash.new, ::String.new
• Style/ExpandPathArguments - ::File.expand_path, ::Pathname.new
• Style/MutableConstant - ::ENV, ::Struct
• Style/Proc - ::Proc.new
• Style/RandomWithOffset - ::Kernel.rand, ::Random.rand
• Style/RedundantException - ::RuntimeError
• Style/RedundantFreeze - ::ENV
• Style/RescueStandardError - ::StandardError
• Style/SignalException - ::Kernel.raise, ::Kernel.fail
• Style/StderrPuts - ::STDERR
• Style/StructInheritance - ::Struct
• Style/SymbolProc - ::Proc.new
• Style/ZeroLengthPredicate - ::File.stat, ::Tempfile.new, ::StringIO.new

Also in Mixin/EnforceSuperclass used by rubocop-rails ApplicationRecord and maybe other gems:

• Detect ::Class.new and Class.new with or without a block.
• Added a spec using a stubbed cop class to include the mixin.

If any of these changes are unwanted or desired to be split out, I'm happy to do so. That is why I have not yet squashed my commits.

I have not added any examples to documentation as they would not aid the reader's understanding of what the cops are looking for.

---

Before submitting the PR make sure the following are checked:

• Wrote good commit messages.
• Commit message starts with [Fix #issue-number] (if the related issue exists).
• Feature branch is up-to-date with master (if not - rebase it).
• Squashed related commits together.
• Added tests.
• Added an entry to the Changelog if the new code introduces user-observable changes. See changelog entry format.
• The PR relates to only one subject with a clear title and description in grammatically correct, complete sentences.
• Run bundle exec rake default. It executes all tests and RuboCop for itself, and generates the documentation.

[bbatsov]

That's one massive PR! The changes look good to me. Thanks for tacking this!

[marcandre]

This is great! I've been noticing the same mistake too, e.g.: rubocop/rubocop-ast#30

I was wondering if having a function for this wouldn't be a good idea:

def_node_matcher :top_level_constant?, "(const {nil? (cbase)} %1)"

# example of use:
def_node_matcher :unsorted_dir_each?, <<~PATTERN
    (send (send (top_level_constant?(:Dir) {:[] :glob} ...) :each)
PATTERN

We could also add an internal cop against use of (const nil? ...)

[bbatsov]

We could also add an internal cop against use of (const nil? ...)

Good idea!

[marcandre]

So should we merge this as is, or introduce top_level_constant? ?

[bbatsov]

So should we merge this as is, or introduce top_level_constant? ?

I think we should add the helper method, but we can do this after this PR has been merged. I was thinking of cutting a new release today, so I can squeeze this PR in it.

[marcandre]

I'm 👍 to merge as is.

Are you sure you want to release another 0.x with the Cop refactoring merged in? Or if you were thinking of 1.0.0rc, there are a few issues still waiting for your decision that would be great to resolve first

[bbatsov]

Are you sure you want to release another 0.x with the Cop refactoring merged in?

We better get any bad feedback before 1.0. :-)

Or if you were thinking of 1.0.0rc, there are a few issues still waiting for your decision that would be great to resolve first

Probably we should open some ticket with everything outstanding, as I might have missed some of those. Anyways, we'll figure it out.

[biinari]

That's one massive PR!

Sorry it might have been big to look at but it was just repetitions of the same thing in multiple cops (and some spec coverage).

Thanks @bbatsov and @marcandre for reviewing and merging so quickly. Hope everything is going smoothly towards the 1.0 release.