Skip to content
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 cop Lint/OrAssignmentToConstant #9363

Merged

Conversation

uplus
Copy link
Contributor

@uplus uplus commented Jan 11, 2021

This cop checks unintended or-assignment to constant.
Usually, you don't need to consider whether A is already assigned elsewhere.

# bad
CONST ||= 1

# good
CONST = 1

Before submitting the PR make sure the following are checked:

  • The PR relates to only one subject with a clear title and description in grammatically correct, complete sentences.
  • 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.
  • Ran bundle exec rake default. It executes all tests and runs RuboCop on its own code.
  • Added an entry (file) to the changelog folder named {change_type}_{change_description}.md if the new code introduces user-observable changes. See changelog entry format for details.

@uplus uplus force-pushed the add-new-cop-or_assignment_to_constant branch 2 times, most recently from 89e3ad8 to 18089b8 Compare January 11, 2021 14:58
Copy link
Member

@dvandersluis dvandersluis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like a useful cop!

lib/rubocop/cop/lint/or_assignment_to_constant.rb Outdated Show resolved Hide resolved
spec/rubocop/cop/lint/or_assignment_to_constant_spec.rb Outdated Show resolved Hide resolved
lib/rubocop/cop/lint/or_assignment_to_constant.rb Outdated Show resolved Hide resolved
@uplus uplus force-pushed the add-new-cop-or_assignment_to_constant branch from ad16e68 to 6d8fc1a Compare January 13, 2021 08:19
@uplus
Copy link
Contributor Author

uplus commented Jan 13, 2021

Thank you for review.
I fixed the three places you pointed out, resolved conflicts, and squashed them.

This cop checks for unintended or-assignment to a constant.

Constants should always be assigned in the same location. And its value
should always be the same. If constants are assigned in multiple
locations, the result may vary depending on the order of `require`.

Also, if you already have such an implementation, auto-correction may
change the result.

```ruby
CONST ||= 1

CONST = 1
```
@uplus uplus force-pushed the add-new-cop-or_assignment_to_constant branch from c31fdf9 to 93a7e0d Compare January 14, 2021 05:05
Copy link
Member

@dvandersluis dvandersluis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks 👍

@rubocop-hq/rubocop-core ci/circleci: cc-upload-coverage keeps failing on this PR, anyone have any ideas why?

@koic
Copy link
Member

koic commented Jan 15, 2021

ci/circleci: cc-upload-coverage keeps failing on this PR, anyone have any ideas why?

It's possible that Code Climate's API wasn't working properly. I think this failure can be ignored.

#!/bin/bash -eo pipefail
./tmp/cc-test-reporter sum-coverage tmp/codeclimate.*.json --parts 6 --output tmp/codeclimate.total.json
./tmp/cc-test-reporter upload-coverage --input tmp/codeclimate.total.json
Error: response from https://api.codeclimate.com/v1/test_reports.
HTTP 401: You are not authorized for this action
Usage:
  cc-test-reporter upload-coverage [flags]

Flags:
  -s, --batch-size int    batch size for source files (default 500)
  -e, --endpoint string   endpoint to upload coverage information to (default "https://api.codeclimate.com/v1/test_reports")
  -r, --id string         reporter identifier (default "a11b66bfbb1acdf220d5cb317b2e945a986fd85adebe29a76d411ad6d74ec31f")
  -i, --input string      input path (default "coverage/codeclimate.json")
      --insecure          send coverage insecurely (without HTTPS)

Global Flags:
  -d, --debug   run in debug mode


Exited with code exit status 255
CircleCI received exit code 255

@dvandersluis
Copy link
Member

@koic yeah that’s what I figured but I wanted to double check, thanks.

@dvandersluis dvandersluis merged commit b72ae11 into rubocop:master Jan 15, 2021
@uplus uplus deleted the add-new-cop-or_assignment_to_constant branch January 15, 2021 15:50
jmkoni pushed a commit to standardrb/standard that referenced this pull request May 3, 2021
* Update rubocop from 1.12.1 to [1.13.0](https://github.com/rubocop-hq/rubocop/releases/tag/v1.13.0)
* Update rubocop-performance from 1.9.2 to [1.11.1](https://github.com/rubocop-hq/rubocop-performance/releases/tag/v1.11.1)
* Enabled the following rules:
  * [`Performance/RedundantSplitRegexpArgument`](rubocop/rubocop-performance#190)
  * [`Style/IfWithBooleanLiteralBranches`](rubocop/rubocop#9396)
  * [`Lint/TripleQuotes`](rubocop/rubocop#9402)
  * [`Lint/SymbolConversion`](rubocop/rubocop#9362)
  * [`Lint/OrAssignmentToConstant`](rubocop/rubocop#9363)
  * [`Lint/NumberedParameterAssignment`](rubocop/rubocop#9326)
  * [`Style/HashConversion`](rubocop/rubocop#9478)
  * [`Gemspec/DateAssignment`](rubocop/rubocop#9496)
  * [`Style/StringChars`](rubocop/rubocop#9615)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants