-
-
Notifications
You must be signed in to change notification settings - Fork 3.1k
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
Add new Lint/DeprecatedConstants
cop
#9324
Conversation
7df54a1
to
abe9fc4
Compare
Nice cop, but it might also be a good idea to add some data about the Ruby version where something got deprecated to the config as well. This will allow us to have more informative messages. |
abe9fc4
to
dbdef11
Compare
That's good idea! I added deprecated version for deprecated constants. |
|
||
def message(good, bad, deprecated_version) | ||
if deprecated_version | ||
deprecated_message = ", which is deprecated in Ruby #{deprecated_version}" |
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.
"deprecated since" would be a slightly better wording here
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.
I see! Thank your for the suggestion!
dbdef11
to
32915a2
Compare
config/default.yml
Outdated
'NIL': ['nil', '2.4'] | ||
'TRUE': ['true', '2.4'] | ||
'FALSE': ['false', '2.4'] | ||
'Random::DEFAULT': ['Random.new', '3.0'] |
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.
I'm not sure whether array is good. For example, hash may be preferable. Please let me know if you have feedback 😅
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.
I agree that hash would be better. This would spare us from having to use explicit null
s.
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.
This would spare us from having to use explicit
null
s.
Sure! That makes sense :-) I updated it!
32915a2
to
f33ca14
Compare
This PR adds new `Lint/DeprecatedConstants` cop. This cop checks for deprecated constants. It has `DeprecatedConstants` config. If there is an alternative method, you can set alternative value as `Alternative`. And you can set the deprecated version as `DeprecatedVersion`. These options can be omitted if they are not needed. By default, `NIL`, `TRUE`, `FALSE` and `Random::DEFAULT` are configured. ```ruby # bad NIL TRUE FALSE Random::DEFAULT # Return value of Ruby 2 is `Random` instance, Ruby 3.0 is `Random` class. # good nil true false Random.new # `::DEFAULT` has been deprecated in Ruby 3, `.new` is compatible with Ruby 2. ``` The following are these sources of information. - Remove (deprecated) `NIL`, `TRUE`, and `FALSE` ... ruby/ruby@62554ca - Deprecate `Random::DEFAULT` ... https://bugs.ruby-lang.org/issues/17351
f33ca14
to
b95afa7
Compare
Thanks! |
This PR adds new
Lint/DeprecatedConstants
cop.This cop checks for deprecated constants.
It has
DeprecatedConstants
option. If there is an alternative method, you can set the value as a string. Or setnull
to the value if there is no alternative.By default,
NIL
,TRUE
,FALSE
andRandom::DEFAULT
are configured.The following are these sources of information.
NIL
,TRUE
, andFALSE
... ruby/ruby@62554caRandom::DEFAULT
... https://bugs.ruby-lang.org/issues/17351Before 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 runs RuboCop on its own code.{change_type}_{change_description}.md
if the new code introduces user-observable changes. See changelog entry format for details.