KEMBAR78
[Fix #14475] Fix cop errors during autocorrect for the buildin LSP when analyzing as Ruby 3.4 by Earlopain · Pull Request #14478 · rubocop/rubocop · GitHub
Skip to content

Conversation

@Earlopain
Copy link
Contributor

@Earlopain Earlopain commented Aug 25, 2025

Fix #14475

After autocorrect happened, it still reused it the parse result from the initial file, causing all kind of trouble.

cc @vinistock


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.

# rubocop:disable Metrics/AbcSize, Metrics/MethodLength
def with_server(
source = nil, path = 'fake.rb', pwd: '..', stub_no_typechecker: false, load_addons: true
source = nil, path = 'fake.rb', stub_no_typechecker: false, load_addons: true
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I removed the pwd option. This test is already isolated and going one dir down interferes with the .rubocop.yml created in the test

RUBY
end

context 'with prism as the parser' do
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This test is very specific but I couldn't come up with a better one. This does run into an infinite loop before this change

@Earlopain Earlopain force-pushed the lsp-autocorrect-prism-reuse branch 2 times, most recently from b8d792d to d011636 Compare August 25, 2025 21:08
… LSP when analyzing as Ruby 3.4

After autocorrect happened, it still reused it the parse result from the initial file,
causing all kind of trouble.
@Earlopain Earlopain force-pushed the lsp-autocorrect-prism-reuse branch from d011636 to 609a4a0 Compare August 25, 2025 21:29
@Earlopain Earlopain changed the title [Fix #14473] Fix cop errors during autocorrect for the buildin LSP when analyzing as Ruby 3.4 [Fix #14475] Fix cop errors during autocorrect for the buildin LSP when analyzing as Ruby 3.4 Aug 25, 2025
@bbatsov
Copy link
Collaborator

bbatsov commented Aug 26, 2025

Your changes seem good to me. Thanks!

@bbatsov bbatsov merged commit d7972b6 into rubocop:master Aug 26, 2025
22 checks passed
Earlopain added a commit to Earlopain/ruby-lsp that referenced this pull request Aug 27, 2025
RuboCop 1.80.1 contains a fix for why this was broken,
so it should be safe to enable this again.
Revert: Shopify#3672
Fix: rubocop/rubocop#14478
Earlopain added a commit to Earlopain/ruby-lsp that referenced this pull request Aug 27, 2025
RuboCop 1.80.1 contains a fix for why this was broken,
so it should be safe to enable this again.
Revert: Shopify#3672
Fix: rubocop/rubocop#14478
Earlopain added a commit to Earlopain/ruby-lsp that referenced this pull request Aug 27, 2025
RuboCop 1.80.1 contains a fix for why this was broken,
so it should be safe to enable this again.
Revert: Shopify#3672
Fix: rubocop/rubocop#14478
vinistock pushed a commit to Shopify/ruby-lsp that referenced this pull request Aug 28, 2025
RuboCop 1.80.1 contains a fix for why this was broken,
so it should be safe to enable this again.
Revert: #3672
Fix: rubocop/rubocop#14478
@Earlopain Earlopain deleted the lsp-autocorrect-prism-reuse branch September 3, 2025 12:10
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.

VSCode LSP formatter crashes

2 participants