Skip to content

Fix printing of symbol in CLS when hovering#27582

Merged
jabraham17 merged 17 commits intochapel-lang:mainfrom
jabraham17:fix-hover
Aug 5, 2025
Merged

Fix printing of symbol in CLS when hovering#27582
jabraham17 merged 17 commits intochapel-lang:mainfrom
jabraham17:fix-hover

Conversation

@jabraham17
Copy link
Member

@jabraham17 jabraham17 commented Jul 31, 2025

Fixes an issue where hovering over a primary method would just show this. Now it will show the method name properly. Along the way, I also added the type name for secondary methods (and primary methods when resolution is enabled)

In this PR:

  • added hash and rich comparison to AstNode in chapel-py
  • added is_this to Formal in chapel-py
  • used the new comparison ops for AstNode to improve code quaility
  • fixed hovering over a primary method
  • improved printing of a secondary method by adding the type name
  • improved printing of a primary method by adding the type name (only when resolution is enabled). This only works well when the type/method is fully concrete, because there is no way to compute an instantiation from a hover. In the future, it would be nice to be able to show the generic-ness of the type.
  • fixes an issue with the venv install for test-cls

Before
Screenshot 2025-08-01 at 9 10 26 AM
Screenshot 2025-08-01 at 9 10 17 AM
Screenshot 2025-08-01 at 9 10 12 AM

After
Screenshot 2025-08-01 at 9 10 52 AM
Screenshot 2025-08-01 at 9 10 45 AM
Screenshot 2025-08-01 at 9 10 40 AM

  • make test-cls

[Reviewed by @DanilaFe]

Signed-off-by: Jade Abraham <jade.abraham@hpe.com>
Signed-off-by: Jade Abraham <jade.abraham@hpe.com>
Signed-off-by: Jade Abraham <jade.abraham@hpe.com>
Signed-off-by: Jade Abraham <jade.abraham@hpe.com>
Signed-off-by: Jade Abraham <jade.abraham@hpe.com>
Signed-off-by: Jade Abraham <jade.abraham@hpe.com>
Signed-off-by: Jade Abraham <jade.abraham@hpe.com>
Signed-off-by: Jade Abraham <jade.abraham@hpe.com>
Signed-off-by: Jade Abraham <jade.abraham@hpe.com>
Signed-off-by: Jade Abraham <jade.abraham@hpe.com>
@jabraham17 jabraham17 self-assigned this Aug 1, 2025
Signed-off-by: Jade Abraham <jade.abraham@hpe.com>
Signed-off-by: Jade Abraham <jade.abraham@hpe.com>
@jabraham17 jabraham17 requested a review from DanilaFe August 1, 2025 16:11
Comment on lines 646 to 650
auto parent = chpl::parsing::parentAst(context, node);
return parent && parent->isFunction() &&
parent->toFunction()->thisFormal() &&
parent->toFunction()->thisFormal()->id() == node->id();
)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

parentAst is notoriously expensive. Have you found issues with literally checing if name == USTR("this")?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think there is any issue with that, since the parser prevents it from being used in any Formal name context

Comment on lines 88 to 93
for i in range(len(self._signature)):
tag = self._signature[i].tag
node = self._signature[i].node
value = self._signature[i].value
prefix = self._signature[i].prefix or ""
postfix = self._signature[i].postfix or ""
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you do:

        for (i, part) in enumerate(self._signature):
            tag = part.tag
            node = part.node
            value = part.value
            prefix = part.prefix or ""
            postfix = part.postfix or ""

Then, I suggest in-lining the variables only used a handful of times (looks like tag and value)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I inlined tag (used once), but left value since its twice (same as node)

Signed-off-by: Jade Abraham <jade.abraham@hpe.com>
Signed-off-by: Jade Abraham <jade.abraham@hpe.com>
Signed-off-by: Jade Abraham <jade.abraham@hpe.com>
Signed-off-by: Jade Abraham <jade.abraham@hpe.com>
Signed-off-by: Jade Abraham <jade.abraham@hpe.com>
@jabraham17 jabraham17 merged commit 4ddf1dd into chapel-lang:main Aug 5, 2025
11 checks passed
@jabraham17 jabraham17 deleted the fix-hover branch August 5, 2025 21:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants