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

feat: Allow excluding specific traits from completion #18179

Merged
merged 6 commits into from
Jan 1, 2025

Conversation

ChayimFriedman2
Copy link
Contributor

@ChayimFriedman2 ChayimFriedman2 commented Sep 24, 2024

It should be pretty easy to extend support to excluding only specific methods, but I didn't do that currently.

Traits configured to be excluded are resolved in each completion request from scratch. If this proves too expensive, it is easy enough to cache them in the DB.

Leave #17477 open, as per @dtolnay request.

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Sep 24, 2024
@ChayimFriedman2 ChayimFriedman2 force-pushed the omit-trait-completion branch 2 times, most recently from 61667ba to 1878d26 Compare September 24, 2024 18:13
crates/hir/src/lib.rs Outdated Show resolved Hide resolved
crates/rust-analyzer/src/config.rs Outdated Show resolved Hide resolved
crates/rust-analyzer/src/config.rs Outdated Show resolved Hide resolved
crates/rust-analyzer/src/config.rs Outdated Show resolved Hide resolved
crates/ide-completion/src/context.rs Outdated Show resolved Hide resolved
crates/ide-completion/src/completions/expr.rs Show resolved Hide resolved
@Veykril Veykril added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Sep 25, 2024
crates/hir/src/lib.rs Outdated Show resolved Hide resolved
crates/ide-completion/src/context.rs Outdated Show resolved Hide resolved
crates/rust-analyzer/src/config.rs Outdated Show resolved Hide resolved
@bors
Copy link
Contributor

bors commented Sep 30, 2024

☔ The latest upstream changes (presumably #18167) made this pull request unmergeable. Please resolve the merge conflicts.

@bors
Copy link
Contributor

bors commented Oct 14, 2024

☔ The latest upstream changes (presumably #18291) made this pull request unmergeable. Please resolve the merge conflicts.

@Veykril
Copy link
Member

Veykril commented Dec 24, 2024

So, I am finally coming back to this (apologies, I forgot about it!) and have since seen the PR to IJ that added similar things intellij-rust/intellij-rust#9123.

They have apparently two types of exclusion, they allow excluding completions of only the traits methods, or the methods as well as the trait itself. I think we wanna stick with what we have here for now, but make the config more general. That is instead of completion_autoimport_excludeTraits just completion_autoimport_exclude and then have that take a map instead of a list where the value is the kind of exclusion (which only allows one kind for now). That allows us to re-use this for more things. Similar treatment for completion_excludeTraits -> completion_exclude. Thoughts on that?

@Veykril
Copy link
Member

Veykril commented Dec 31, 2024

Relevant issue #13786 (comment)

To be accurate, only their methods are excluded, the trait themselves are still available.

I also excluded a bunch of std traits by default. Some less opinionated, like `AsRef`, which should never be used directly except in generic scenarios (and won't be excluded there), some more opinionated, like the ops traits, which I know some users sometimes want to use directly. Either way it's configurable.

It should be pretty easy to extend support to excluding only specific methods, but I didn't do that currently.

Traits configured to be excluded are resolved in each completion request from scratch. If this proves too expensive, it is easy enough to cache them in the DB.
… the name

I.e. with `fn foo()`, don't complete at `x.fo|`, but complete (with imports) for `x.foo|`, since this is less likely to have false positives.

I opted to only do that for flyimport, even though for basic imports there can also be snippet completion (completing the params list for a method), since this is less universally applicable and seems not so useful.
@Veykril Veykril force-pushed the omit-trait-completion branch from 1d9543f to a02a1af Compare January 1, 2025 12:57
@Veykril Veykril force-pushed the omit-trait-completion branch from f804cd0 to fc4940f Compare January 1, 2025 14:06
@Veykril Veykril force-pushed the omit-trait-completion branch from fc4940f to 1adc805 Compare January 1, 2025 14:22
@Veykril Veykril enabled auto-merge January 1, 2025 14:22
@Veykril Veykril added this pull request to the merge queue Jan 1, 2025
Merged via the queue into rust-lang:master with commit 7e639ee Jan 1, 2025
9 checks passed
@ChayimFriedman2 ChayimFriedman2 deleted the omit-trait-completion branch January 1, 2025 15:49
@0xangelo
Copy link

0xangelo commented Jan 10, 2025

This doesn't seem to be working. On a simple project with color_eyre as a dependency, I still see suggestions for it
image

I did export RA_LOG=info in the root of my repo, then launched my editor. Looking at the log file for rust-analyzer, I can see that the client sent my config with the desired completion.excludeTraits config. I also shows that the server version is 9fc6b43 2025-01-07, which came later than the release that incorporated this feature.

Any pointers?

rust-analyzer logs
2025-01-10T10:39:18.514172767-03:00  INFO server version 1.84.0 (9fc6b43 2025-01-07) will start
2025-01-10T10:39:18.514521578-03:00  INFO InitializeParams: {"processId":136243,"workDoneToken":"1","workspaceFolders":[{"uri":"file:///home/doom/git/unmaykr-aftermath/playground","name":"/home/doom/git/unmaykr-aftermath/playground"}],"capabilities":{"workspace":{"workspaceEdit":{"resourceOperations":["rename","create","delete"]},"didChangeConfiguration":{"dynamicRegistration":false},"applyEdit":true,"workspaceFolders":true,"semanticTokens":{"refreshSupport":true},"configuration":true,"inlayHint":{"refreshSupport":true},"symbol":{"dynamicRegistration":false,"symbolKind":{"valueSet":[1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,16,17,18,19,20,21,22,23,24,25,26]}},"didChangeWatchedFiles":{"dynamicRegistration":false,"relativePatternSupport":true}},"general":{"positionEncodings":["utf-16"]},"textDocument":{"rename":{"prepareSupport":true,"dynamicRegistration":true},"semanticTokens":{"multilineTokenSupport":false,"overlappingTokenSupport":true,"tokenTypes":["namespace","type","class","enum","interface","struct","typeParameter","parameter","variable","property","enumMember","event","function","method","macro","keyword","modifier","comment","string","number","regexp","operator","decorator"],"serverCancelSupport":false,"tokenModifiers":["declaration","definition","readonly","static","deprecated","abstract","async","modification","documentation","defaultLibrary"],"dynamicRegistration":false,"requests":{"range":false,"full":{"delta":true}},"augmentsSyntaxTokens":true,"formats":["relative"]},"completion":{"insertTextMode":1,"completionList":{"itemDefaults":["editRange","insertTextFormat","insertTextMode","data"]},"dynamicRegistration":false,"contextSupport":false,"completionItemKind":{"valueSet":[1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,16,17,18,19,20,21,22,23,24,25]},"completionItem":{"preselectSupport":true,"insertReplaceSupport":true,"labelDetailsSupport":true,"deprecatedSupport":true,"commitCharactersSupport":true,"snippetSupport":true,"resolveSupport":{"properties":["documentation","detail","additionalTextEdits"]},"tagSupport":{"valueSet":[1]},"insertTextModeSupport":{"valueSet":[1,2]},"documentationFormat":["markdown","plaintext"]}},"inlayHint":{"dynamicRegistration":true,"resolveSupport":{"properties":["textEdits","tooltip","location","command"]}},"definition":{"linkSupport":true,"dynamicRegistration":true},"callHierarchy":{"dynamicRegistration":false},"publishDiagnostics":{"relatedInformation":true,"dataSupport":true,"tagSupport":{"valueSet":[1,2]}},"implementation":{"linkSupport":true},"hover":{"dynamicRegistration":true,"contentFormat":["markdown","plaintext"]},"documentSymbol":{"dynamicRegistration":false,"hierarchicalDocumentSymbolSupport":true,"symbolKind":{"valueSet":[1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,16,17,18,19,20,21,22,23,24,25,26]}},"formatting":{"dynamicRegistration":true},"references":{"dynamicRegistration":false},"signatureHelp":{"dynamicRegistration":false,"signatureInformation":{"parameterInformation":{"labelOffsetSupport":true},"activeParameterSupport":true,"documentationFormat":["markdown","plaintext"]}},"synchronization":{"willSaveWaitUntil":true,"didSave":true,"dynamicRegistration":false,"willSave":true},"typeDefinition":{"linkSupport":true},"foldingRange":{"dynamicRegistration":false,"lineFoldingOnly":true},"rangeFormatting":{"dynamicRegistration":true},"declaration":{"linkSupport":true},"diagnostic":{"dynamicRegistration":false},"codeAction":{"resolveSupport":{"properties":["edit"]},"isPreferredSupport":true,"dynamicRegistration":true,"dataSupport":true,"codeActionLiteralSupport":{"codeActionKind":{"valueSet":["","quickfix","refactor","refactor.extract","refactor.inline","refactor.rewrite","source","source.organizeImports"]}}},"documentHighlight":{"dynamicRegistration":false}},"experimental":{"codeActionGroup":true,"snippetTextEdit":true,"serverStatusNotification":true,"hoverRange":true,"colorDiagnosticOutput":true,"commands":{"commands":["rust-analyzer.runSingle","rust-analyzer.showReferences","rust-analyzer.gotoLocation","editor.action.triggerParameterHints"]},"hoverActions":true,"ssr":true},"window":{"showDocument":{"support":true},"workDoneProgress":true,"showMessage":{"messageActionItem":{"additionalPropertiesSupport":false}}}},"trace":"off","initializationOptions":{"check":{"command":"clippy"},"rustfmt":{"extraArgs":["+nightly"]},"cargo":{"allFeatures":true},"completion":{"excludeTraits":["core::borrow::Borrow","core::borrow::BorrowMut","std::borrow::Borrow","std::borrow::BorrowMut","owo_colors::OwoColorize","color_eyre::owo_colors::OwoColorize"]}},"rootUri":"file:///home/doom/git/unmaykr-aftermath/playground","rootPath":"/home/doom/git/unmaykr-aftermath/playground","clientInfo":{"name":"Neovim","version":"0.10.3"}}
2025-01-10T10:39:18.514697365-03:00  INFO Client 'Neovim' 0.10.3
2025-01-10T10:39:18.514786612-03:00  INFO updating config from JSON: {
  "check": {
    "command": "clippy"
  },
  "rustfmt": {
    "extraArgs": [
      "+nightly"
    ]
  },
  "cargo": {
    "allFeatures": true
  },
  "completion": {
    "excludeTraits": [
      "core::borrow::Borrow",
      "core::borrow::BorrowMut",
      "std::borrow::Borrow",
      "std::borrow::BorrowMut",
      "owo_colors::OwoColorize",
      "color_eyre::owo_colors::OwoColorize"
    ]
  }
}
Nvim config
return {
  {
    "AstroNvim/astrolsp",
    opts = function(_, opts)
      opts.config.rust_analyzer.settings["rust-analyzer"] = {
        check = {
          command = "clippy",
        },

        cargo = {
          allFeatures = true,
        },

        completion = {
          excludeTraits = {
            "core::borrow::Borrow",
            "core::borrow::BorrowMut",
            "std::borrow::Borrow",
            "std::borrow::BorrowMut",
            "owo_colors::OwoColorize",
            "color_eyre::owo_colors::OwoColorize",
          },
        },

        rustfmt = {
          extraArgs = {
            "+nightly",
          },
        },
      }

      return opts
    end,
  },
}

Edit

Later in the logs I see the following, maybe it's helpful

    client_config: (
        FullConfigInput {
            global: GlobalConfigInput,
            workspace: WorkspaceConfigInput {
                cargo_features: All,
                check_command: "clippy",
                rustfmt_extraArgs: [
                    "+nightly",
                ],
            },
            local: LocalConfigInput,
            client: ClientConfigInput,
        },
        ConfigErrors(
            [],
        ),
    ),

@Veykril
Copy link
Member

Veykril commented Jan 10, 2025

Hmm, it does work for me in vscode

@0xangelo
Copy link

For those who are using Neovim and may run into the same problems as I did above, getting rid of rustaceanvim did it for me. I've spent way too much time trying to get it to work with it

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants