New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Detect top-level constant usage like ::Const
#8245
Conversation
Detect `::Class.new` and `Class.new` in EnforceSuperclass mixin, with or without a block.
Detect `::RUBY_VERSION` in Gemspec/RubyVersionGlobalsUsage cop.
Detect `::Class.new`, `::Module.new`, `::Struct.new` in top-level namespace for Lint/NestedMethodDefinition.
Detect top-level constant `::Dir` in Lint/NonDeterministicRequireOrder.
Detect top-level constant `::Kernel.rand(1)` in Lint/RandOne cop.
Detect redundant array splat with top-level constant `::Array`.
Detect useless access modifier in `::Class.new`, `::Module.new`, `::Struct.new` blocks.
Detect `::Class.new`, `::Module.new` in Metrics/ClassLength and Metrics/ModuleLength cops.
Allow historic DateTime using `::Date` constants in Style/DateTime cop.
Detect `::Array.new`, `::Hash.new`, `::String.new` producing empty literals.
Detect `::Proc.new` with a block in Style/Proc.
Detect with top-level constant as `::Kernel.raise` and `::Kernel.fail`
Allow `::File.stat`, `::Tempfile.new`, `::StringIO.new`
Detect `::Random.rand`, `::Kernel.rand` in Style/RandomWithOffset.
Detect `::File.expand_path`, `::Pathname.new` in Style/ExpandPathArguments cop.
Allow top-level `::ENV`, `::Struct` in Style/MutableConstant cop.
Detect top-level constants
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's one massive PR! The changes look good to me. Thanks for tacking this!
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:
We could also add an internal cop against use of |
Good idea! |
So should we merge this as is, or introduce |
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. |
I'm 👍 to merge as is. Are you sure you want to release another |
We better get any bad feedback before 1.0. :-)
Probably we should open some ticket with everything outstanding, as I might have missed some of those. Anyways, we'll figure it out. |
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. |
[Fix #8215]
Detect top-level constant usage (in offenses or allowed forms) like
::Const
in the following cops:::RUBY_VERSION
::Class.new
,::Module.new
,::Struct.new
::Dir
::Kernel.rand(1)
::Array.new
::NotImplementedError
::Class.new
,::Module.new
,::Struct.new
::Class.new
::Module.new
::Date
constants for historic date::File
::Array.new
,::Hash.new
,::String.new
::File.expand_path
,::Pathname.new
::ENV
,::Struct
::Proc.new
::Kernel.rand
,::Random.rand
::RuntimeError
::ENV
::StandardError
::Kernel.raise
,::Kernel.fail
::STDERR
::Struct
::Proc.new
::File.stat
,::Tempfile.new
,::StringIO.new
Also in
Mixin/EnforceSuperclass
used by rubocop-rails ApplicationRecord and maybe other gems:::Class.new
andClass.new
with or without a block.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:
[Fix #issue-number]
(if the related issue exists).master
(if not - rebase it).bundle exec rake default
. It executes all tests and RuboCop for itself, and generates the documentation.