microsoft / vscode-cpptools

Official repository for the Microsoft C/C++ extension for VS Code.
Other
5.54k stars 1.56k forks source link

Add clang-tidy `editor.codeActionsOnSave` entry point #12684

Open thernstig opened 2 months ago

thernstig commented 2 months ago

Feature Request

The proper way for extensions to add linting fixes (not formatting fixes) is to add entries like this in the VS Code config:

  "editor.codeActionsOnSave": {
    "source.organizeImports": "always",
    "source.fixAll.eslint": "always",
    "source.fixAll.markdownlint": "always",
    "source.fixAll.stylelint": "always"
  },

It would be nice if this extension could add a source.fixAll.clangTidy that would then automatically fix all fixable problems on save.

References

https://stackoverflow.com/a/61051832/1853417 https://code.visualstudio.com/api/references/vscode-api (search for CodeActionProvider) https://code.visualstudio.com/updates/v1_83#_code-actions-on-save-and-auto-save

github-actions[bot] commented 3 weeks ago

This feature request is being closed due to insufficient upvotes. Please leave a 👍-upvote or 👎-downvote reaction on the issue to help us prioritize it. When enough upvotes are received, this issue will be eligible for our backlog.

thernstig commented 3 weeks ago

@sean-mcmanus it seems impossible to get enough upvotes when there is so little activity in the repo and only a 2 month time for something to be upvoted.

This is also the defacto standard for how to handle this from VS Code, almost to the point that this should be considered a bug.

bobbrow commented 3 weeks ago

@thernstig, please don't take it personally, we add the "more votes needed" tag to highlight that this feature request can be considered in the future and the bot will reopen it automatically when the votes are received. With over 1000 active issues and a very small team, we have to set some rules for feature requests, not only to help us manage the overall backlog size, but also to set expectations about the chances that something may make it into the product.

We do have a modest amount of activity in the repo, but we also set the bar very low for the number of upvotes required. The VS Code team requires 20. We only require 3 and we allow the requestor to vote on their own request.

sean-mcmanus commented 3 weeks ago

@thernstig What exactly do these statements mean: add entries like this in the VS Code config and extension could add a source.fixAll.clangTidy mean? Is something you could add via modifying our TypeScript implementation? Do you know of any examples of this being added for other extensions? i.e. I'd have to do research into how that would be implemented.

thernstig commented 3 weeks ago

@bobbrow oh, thanks @bobbrow 😄. Understandable. I am not as angry as I might sound. I am just a huge DX proponent, and I know how many other languages extensions handles this in VS Code, that feel more modern in key areas around how, as far as my understanding is, should be done. This extensions is great though! I am and want to continue to use it very much, and am merely striving to lift some key areas that I find most language extensions should have (again, from my own experience). Maybe I should send in a resume to see if I can join the team instead of writing issues 😄🤚

@sean-mcmanus see https://github.com/microsoft/vscode-eslint?tab=readme-ov-file#version-204 and https://github.com/astral-sh/ruff-vscode?tab=readme-ov-file#configuring-vs-code for two very large extensions, for the huge languages JavaScript and Python.

sean-mcmanus commented 3 weeks ago

@thernstig I looked at those examples repos, but I still don't see what API they're calling to register a particular fixit values like "source.fixAll.clangTidy". Maybe we can just check the editor.codeActionsOnSave for "source.fixAll" or "source.fixAll.clangTidy", but I don't think it would appear as a completion option in the settings UI.

thernstig commented 3 weeks ago

@sean-mcmanus https://code.visualstudio.com/api/references/vscode-api#CodeActionKind maybe?

It is the https://code.visualstudio.com/api/references/vscode-api and CodeActionProvider that is used that:

Change applied on save by the editor.codeActionsOnSave setting.

This all started way back here the discussion around this: https://github.com/microsoft/vscode/issues/47621

sean-mcmanus commented 3 weeks ago

@thernstig Yeah, thanks, that looks like it's it -- we currently use the QuickFix type. I can change it to a new kind to see if it works. I'm not sure if it'll correctly run the formatter after the fix and before saving though.

thernstig commented 3 weeks ago

@sean-mcmanus please note this is only for fixing linting issues that are auto-fixable, such as for clang-tidy. It is not supposed to be used for formatters like clang-format.

I.e. it is supposed to be used for auto-fixable problems as listed here: https://clang.llvm.org/extra/clang-tidy/checks/list.html

bobbrow commented 3 weeks ago

Maybe I should send in a resume to see if I can join the team instead of writing issues 😄🤚

😄 Unfortunately, we aren't currently hiring, but I think I see you on Linkedin. I'll send you a request to connect.

sean-mcmanus commented 3 weeks ago

@thernstig The formatter generally needs to run on the fixit's or they don't have any alignment.

sean-mcmanus commented 1 week ago

@thernstig With

    "C_Cpp.codeAnalysis.clangTidy.checks.enabled": ["readability-make-member-function-const", "readability-const-return-type"],
    "editor.codeActionsOnSave": {
        "source.fixAll": "always"
    }

and

class foo
{
int i;
public:
  int func() { return i; }
  int func2() { return i; }
  const int func3() { return 0; }
};

and changing the QuickFix type to SourceFixAll at https://github.com/microsoft/vscode-cpptools/blob/main/Extension/src/LanguageServer/codeAnalysis.ts#L127

It causes the "Fix all" option to disappear from the Quick Fix list (we could probably work around that by adding a different code action for that type), but it never invokes the C_Cpp.FixAllCodeAnalysisProblems command, so either I'm not doing something correct or there's some VS Code bug. I get the same result when I use const codeAnalysisFixAllMacroKind: vscode.CodeActionKind = vscode.CodeActionKind.SourceFixAll.append("clangTidy"); as the type.

thernstig commented 5 days ago

@sean-mcmanus I have not tested this myself so I cannot give an exact answer, but there are other extensions that do this. See e.g. https://marketplace.visualstudio.com/items?itemName=dbaeumer.vscode-eslint and https://marketplace.visualstudio.com/items?itemName=charliermarsh.ruff that does this. Their code base should give some hint. (I would check it if I had the time, but I am swamped atm).

In addition, there should be a source.fixAll.clangTidy added. Below is an example of a real project of mine, where each extension/linter has its own config. Of course a source.fixAll should still work, enabling all linters that have a editor.codeActionsOnSave -> source.fixAll hook.

{
  "editor.codeActionsOnSave": {
    "source.organizeImports": "always",
    "source.fixAll.eslint": "always",
    "source.fixAll.markdownlint": "always",
    "source.fixAll.stylelint": "always"
  }
}