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

Corresponding code actions for diagnostics #39

Open
4 of 8 tasks
figsoda opened this issue Dec 4, 2022 · 5 comments
Open
4 of 8 tasks

Corresponding code actions for diagnostics #39

figsoda opened this issue Dec 4, 2022 · 5 comments
Labels
A-assist Area: assistence/code action C-feature Catagory: feature good first issue Good for newcomers

Comments

@figsoda
Copy link
Contributor

figsoda commented Dec 4, 2022

List from DiagnosticKind

  • SyntaxError

Lowering

Name resolution

Liveness

  • UnusedBinding
  • UnusedWith
  • UnusedRec
@oxalica oxalica added C-feature Catagory: feature A-assist Area: assistence/code action labels Dec 4, 2022
@oxalica
Copy link
Owner

oxalica commented Dec 4, 2022

Note that we can get back diagnostics from Code Actions requests. This would make it easier to implement without duplicate checking codes.

  • DuplicatedParam
  • MergePlainRecAttrset
  • MergeRecAttrset

These should be left as-is and let user check what is happening.

  • Unused*

These could occur as false-positives during editing. Delete actions would hurt by accidental application. As a reference, rust-analyzer provides an action for if this is intentional, prefix it with an underscore: `_a`, and no actions to delete the binding. But we currently have no similar notations to suppress the warnings.

@figsoda
Copy link
Contributor Author

figsoda commented Dec 4, 2022

These should be left as-is and let user check what is happening.

I crossed these out now

Unused*

I think it makes sense for rust to do that due to the impurities, but for nix there shouldn't be a reason to have dead code
I also think that code actions should be less conservative than diagnostics by default since you have to manually trigger them, and there are no convert_to_inherit diagnostics for that reason

@oxalica
Copy link
Owner

oxalica commented Dec 6, 2022

UriLiteral

This fix could be implemented together with string rewriting under the name rewrite_string,

  • URI -> "-string
  • "-string <-> ''-string
  • Identifier <-> "-string (Attr position)

@oxalica oxalica added the good first issue Good for newcomers label Dec 6, 2022
@asymmetric
Copy link

@oxalica I'm interested in trying to implement the UrlLiteral code action. Do you have any more guidance than the post above?

Also happy to chat on Matrix, if that's your thing.

@oxalica
Copy link
Owner

oxalica commented Dec 15, 2022

@asymmetric

@oxalica I'm interested in trying to implement the UrlLiteral code action. Do you have any more guidance than the post above?

You can follow other PRs like #41. For string content, there are some helper functions for unescaping in syntax::semantic.

pub fn unescape_string<E>(

Also happy to chat on Matrix, if that's your thing.

I attached my Matrix address in my profile page. But I might not be quite responsive recently.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-assist Area: assistence/code action C-feature Catagory: feature good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

3 participants