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

Return global offenses for Naming/FileName and Naming/InclusiveLanguage #12802

Merged
merged 1 commit into from Mar 21, 2024

Conversation

Earlopain
Copy link
Contributor

When checking against filenames, a global offense should be added. The current behaviour causes problems in things like LSPs that inspect the offense for location information since some operations on these offenses will raise a IndexError.

I've tweaked the specs for InclusiveLanguage since they just passed without any changes. With the annotation expectations, it actually checks the offense location (or lack thereof).

require "rubocop"

processed_source = RuboCop::ProcessedSource.new("", 3.3)
range = Class.new.extend(RuboCop::Cop::RangeHelp).send(:source_range, processed_source.buffer, 1, 0)
offense = RuboCop::Cop::Offense.new(:info, range, "global offense", "Naming/FileName", :unsupported)
offense.highlighted_area.begin_pos
=> # .../gems/parser-3.3.0.5/lib/parser/source/buffer.rb:274:in `fetch': index 0 outside of array bounds: 0...0 (IndexError)

As a note, Style/Copyright can add a offense in exactly the same way. However, since it supports autocorrect just switching to add_global_offense is not possible.


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.

config/default.yml Outdated Show resolved Hide resolved
@Earlopain Earlopain force-pushed the global-offenses-for-filename-issues branch from 9ae5118 to 1468845 Compare March 21, 2024 07:18
…guage`

Having these return something with a location is problematic because files can be empty.

This leads to `IndexError` for 3rd-party consumers when accessing location information.
@Earlopain Earlopain force-pushed the global-offenses-for-filename-issues branch from 1468845 to ade65de Compare March 21, 2024 07:28
@koic koic merged commit 41758a5 into rubocop:master Mar 21, 2024
33 checks passed
@koic
Copy link
Member

koic commented Mar 21, 2024

Thanks!

@Earlopain Earlopain deleted the global-offenses-for-filename-issues branch March 21, 2024 08:51
Earlopain added a commit to Earlopain/rubocop that referenced this pull request Mar 21, 2024
Very similar to rubocop#12802

This adds a check that the returned offenses have a location that formatters can display.
While the standard formatter catched the `IndexError`, barely any others did.

Since it is not possible to autocorrect global offenses, just don't do that. Adding autocorrect support would be a bunch more work.
bbatsov pushed a commit that referenced this pull request Mar 25, 2024
Very similar to #12802

This adds a check that the returned offenses have a location that formatters can display.
While the standard formatter catched the `IndexError`, barely any others did.

Since it is not possible to autocorrect global offenses, just don't do that. Adding autocorrect support would be a bunch more work.
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.

None yet

2 participants