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

Mark recursive references in let bindings #2902

Open
roberth opened this issue May 11, 2022 · 9 comments
Open

Mark recursive references in let bindings #2902

roberth opened this issue May 11, 2022 · 9 comments
Labels
component: semantic-tokens level: easy The issue is suited for beginners type: enhancement New feature or request

Comments

@roberth
Copy link
Contributor

roberth commented May 11, 2022

Is your feature request related to a problem? Please describe.

I just spent an hour debugging a hanging test, doubting my production code instead of checking the test code 🤦. If the recursive reference were highlighted, I would have found the mistake while writing the code.

For context, here's how it looked:

 do
-  let attr = r & findJust "hello attribute" \case EvaluateEvent.Attribute a | AttributeEvent.expressionPath attr == ["hello"] -> Just a; _ -> Nothing
+  let attr = r & findJust "hello attribute" \case EvaluateEvent.Attribute a | AttributeEvent.expressionPath a == ["hello"] -> Just a; _ -> Nothing
   evaluate attr -- debug hang

Describe the solution you'd like

Highlight recursive references with let exprs and let statements. I expect such a highlight to help with code understanding as well.

  let a = f a
            ^---- highlight this in IDE

Recursive references usually aren't mistakes, so the highlight should use a neutral style. Maybe underline it?

Ideally, the analysis also finds more indirect examples:

  let a = b
      b = g a
            ^---- highlight this in IDE

I suppose mutually recursive functions would also be highlighted by such an analysis. I don't know if this is desirable.

Describe alternatives you've considered

  • Build a cabin in the woods and earn with AirBnB.

  • Add non-recursive let to the language and change habits to use that instead. Keep making mistakes in library code that refuses to use it for compatibility reasons.

Additional context

@michaelpj
Copy link
Collaborator

This is a cool idea. I'm not sure how we'd do it, though. Semantic highlighting, maybe? But we'd need a special token modifier, and I don't think it's easy to get that to work 🤔

@July541
Copy link
Collaborator

July541 commented May 16, 2022

We can use hie to get the recursive relation. Semantic highlighting looks very suitable, but needs some extra work indeed to adapt recursive definition. Maybe we can do it after we have the support of semantic highlighting.

@July541
Copy link
Collaborator

July541 commented May 16, 2022

@michaelpj
Copy link
Collaborator

I guess it could be a diagnostic. It feels noisy to put this information in there: it's going to clutter up people's diagnostic lists, and it's only something you care about occasionally, so that feels kind of bad?

@July541
Copy link
Collaborator

July541 commented May 17, 2022

Yes, information severity is noisy indeed. Maybe we can use hint severity? it won't show in people's diagnostic lists, and it is a hint for users in a way.

@michaelpj
Copy link
Collaborator

@sloorush one to think about when we're thinking about semantic highlighting.

@wz1000 wz1000 added level: easy The issue is suited for beginners and removed status: needs triage labels Sep 7, 2022
@michaelpj
Copy link
Collaborator

cc @soulomoon , although I don't think there's a good token modifier for this, sadly.

@soulomoon
Copy link
Collaborator

soulomoon commented Jan 16, 2024

For the direct recursive case, it should be easy
For the mutual recursive case, we might need to do a dependency graph. But there is a good chance such an analyze tool already exists, we can cache and use the analyzation?

@michaelpj
Copy link
Collaborator

It's also not clear that there is any appropriate type or modifier to use here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: semantic-tokens level: easy The issue is suited for beginners type: enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

5 participants