Skip to content

Conversation

@catlee
Copy link
Contributor

@catlee catlee commented Jun 21, 2023

Motivation

Rubocop will raise a LoadError when it can't resolve requirements listed in .rubocop.yml.

Currently these exceptions are missed, and formatting silently fails.

Implementation

We should catch LoadError in addition to StandardError

Automated Tests

Manual Tests

I can only reproduce this in one internal repo when using nvim & the ruby_ls plugin. I haven't managed to create a small test case for this yet.

I think it has something to do with in-repo gems. In my case the RuboCopRunner raises a LoadError somewhere in find_target_files when trying to load the in-repo gem listed in the require section of .rubocop.yml

@catlee catlee requested a review from a team as a code owner June 21, 2023 14:51
@catlee catlee requested review from Morriar and egiurleo June 21, 2023 14:51
@github-actions
Copy link
Contributor

github-actions bot commented Jun 21, 2023

Benchmark results in seconds (slowest at top)

          textDocument/completion average: 0.263039 std_dev: 0.004947
          textDocument/diagnostic average: 0.04049 std_dev: 0.008738
      textDocument/selectionRange average: 0.003243 std_dev: 0.000357
 textDocument/semanticTokens/full average: 0.001727 std_dev: 0.000173
      textDocument/documentSymbol average: 0.001695 std_dev: 7.1e-05
   textDocument/documentHighlight average: 0.001669 std_dev: 0.000117
        textDocument/documentLink average: 0.001649 std_dev: 0.000127
            textDocument/codeLens average: 0.001646 std_dev: 0.000104
        textDocument/foldingRange average: 0.001433 std_dev: 8.9e-05
textDocument/semanticTokens/range average: 0.000899 std_dev: 6.1e-05
           textDocument/inlayHint average: 0.000575 std_dev: 4.2e-05
               codeAction/resolve average: 0.000562 std_dev: 6.8e-05
               textDocument/hover average: 0.000528 std_dev: 6.2e-05
    textDocument/onTypeFormatting average: 9.7e-05 std_dev: 4.8e-05
          textDocument/formatting average: 6.0e-05 std_dev: 0.000277
          textDocument/codeAction average: 3.2e-05 std_dev: 2.8e-05


================================================================================
Comparison with main branch:

 textDocument/semanticTokens/full unchanged
textDocument/semanticTokens/range slower by 9.891 %
      textDocument/documentSymbol slower by 5.802 %
        textDocument/foldingRange unchanged
          textDocument/formatting unchanged
          textDocument/diagnostic unchanged
        textDocument/documentLink unchanged
           textDocument/inlayHint unchanged
      textDocument/selectionRange unchanged
   textDocument/documentHighlight unchanged
               textDocument/hover unchanged
          textDocument/codeAction unchanged
    textDocument/onTypeFormatting unchanged
               codeAction/resolve unchanged
          textDocument/completion unchanged
            textDocument/codeLens unchanged


At least one benchmark is slower than the main branch.

@vinistock
Copy link
Member

Looks good to me, but can we add the same to diagnostics? Since it also uses RuboCop, it's also failing silently. There's an identical branch to this one a few lines below.

Rubocop will raise a `LoadError` when it can't resolve requirements listed in `.rubocop.yml`.

Currently these exceptions are missed, and formatting silently fails.
@catlee catlee force-pushed the formatting_loaderror branch from 874f982 to 653a322 Compare June 21, 2023 15:49
@vinistock vinistock added the bugfix This PR will fix an existing bug label Jun 21, 2023
@vinistock vinistock merged commit e6641c9 into main Jun 21, 2023
@vinistock vinistock deleted the formatting_loaderror branch June 21, 2023 17:39
@shopify-shipit shopify-shipit bot temporarily deployed to production June 23, 2023 15:44 Inactive
vinistock pushed a commit that referenced this pull request Feb 28, 2024
…sce-2.21.0

Bump @vscode/vsce from 2.20.1 to 2.21.0
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bugfix This PR will fix an existing bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants