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: distinguish highlight for function symbol and normal var symbol #1386

Merged
merged 4 commits into from
Jun 3, 2024

Conversation

shruti2522
Copy link
Contributor

@shruti2522 shruti2522 commented May 31, 2024

1. Does this PR affect any open issues?(Y/N) and add issue references (e.g. "fix #123", "re #123".):

2. What is the scope of this PR (e.g. component or file name):

kclvm/sema/src/core/symbol.rs

3. Provide a description of the PR(e.g. more details, effects, motivations or doc link):

  • Affects user behaviors
  • Contains syntax changes
  • Contains variable changes
  • Contains experimental features
  • Performance regression: Consumes more CPU
  • Performance regression: Consumes more Memory
  • Other

4. Are there any breaking changes?(Y/N) and describe the breaking changes(e.g. more details, motivations or doc link):

  • N
  • Y

5. Are there test cases for these changes?(Y/N) select and add more details, references or doc links:

  • Unit test
  • Integration test
  • Benchmark (add benchmark stats below)
  • Manual test (add detailed scripts or steps below)
  • Other

@shruti2522 shruti2522 force-pushed the highlight branch 2 times, most recently from 4ecfd21 to 063b1ca Compare May 31, 2024 21:58
@coveralls
Copy link
Collaborator

coveralls commented May 31, 2024

Pull Request Test Coverage Report for Build 9352165444

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

Details

  • 121 of 196 (61.73%) changed or added relevant lines in 7 files are covered.
  • 1 unchanged line in 1 file lost coverage.
  • Overall coverage decreased (-0.02%) to 71.086%

Changes Missing Coverage Covered Lines Changed/Added Lines %
kclvm/sema/src/core/global_state.rs 20 21 95.24%
kclvm/tools/src/LSP/src/document_symbol.rs 0 1 0.0%
kclvm/tools/src/LSP/src/semantic_token.rs 4 6 66.67%
kclvm/sema/src/core/symbol.rs 77 148 52.03%
Files with Coverage Reduction New Missed Lines %
kclvm/tools/src/LSP/src/semantic_token.rs 1 90.38%
Totals Coverage Status
Change from base Build 9349800499: -0.02%
Covered Lines: 54991
Relevant Lines: 77358

💛 - Coveralls

@shruti2522 shruti2522 changed the title feat: added FunctionSymbol to distinguish from ValueSymbol feat: distinguish highlight for function symbol and normal var symbol Jun 1, 2024
@shruti2522 shruti2522 force-pushed the highlight branch 2 times, most recently from 8ca81ac to 5e01c58 Compare June 2, 2024 00:36
@shruti2522
Copy link
Contributor Author

shruti2522 commented Jun 2, 2024

  • I have implemented highlight for builtin function calls:

Screenshot from 2024-06-02 20-24-36

  • For lambda expression function calls, I modified the walk_call_expr to handle the function call as a FunctionSymbol. But they are still interpreted as value symbols. Do we need to modify the return type of lambda_expr to be interpreted as function symbol?

Can you please review the changes, @Peefy

@Peefy
Copy link
Contributor

Peefy commented Jun 3, 2024

  • I have implemented highlight for builtin function calls:

Screenshot from 2024-06-02 20-24-36

  • For lambda expression function calls, I modified the walk_call_expr to handle the function call as a FunctionSymbol. But they are still interpreted as value symbols. Do we need to modify the return type of lambda_expr to be interpreted as function symbol?

Can you please review the changes, @Peefy

Thanks. @He1pa and @Peefy will review it.

@He1pa
Copy link
Contributor

He1pa commented Jun 3, 2024

  • I have implemented highlight for builtin function calls:

Screenshot from 2024-06-02 20-24-36

  • For lambda expression function calls, I modified the walk_call_expr to handle the function call as a FunctionSymbol. But they are still interpreted as value symbols. Do we need to modify the return type of lambda_expr to be interpreted as function symbol?

Can you please review the changes, @Peefy

no, you need to update the lambda expr to a function symbol first, then update call expr, e.g.,

func = lambda x: int, y: int -> int {
    x + y
}
a = func(1, 1)  # 2

you need to update the symbol func in line 1, and call expr func(1, 1) in line 4
So, maybe you need to update and walk_assign_stmt() in namer

fn walk_assign_stmt(&mut self, assign_stmt: &'ctx ast::AssignStmt) -> Self::Result {

and walk_identifier_expr() in advanced_resolver

pub fn walk_identifier_expr(

@shruti2522
Copy link
Contributor Author

shruti2522 commented Jun 3, 2024

updated the assign_stmt in namer and resolve_names in identifier_expr to resolve lambda expressions

Screenshot from 2024-06-03 09-22-28

@Peefy
Copy link
Contributor

Peefy commented Jun 3, 2024

@He1pa @shruti2522

I don't think we should call the corresponding syntax nodes such as walk_assign_stmt, walk_call, and walk_identifier_expr etc. It may not cover all cases, but it should be a semantically related function. I believe that simply based on SymbolKind::Value and function type, we can just do it here https://github.com/kcl-lang/kcl/blob/main/kclvm/tools/src/LSP/src/semantic_token.rs#L35

@Peefy
Copy link
Contributor

Peefy commented Jun 3, 2024

PR conflicted.

@shruti2522 shruti2522 marked this pull request as ready for review June 3, 2024 13:47
Signed-off-by: shruti2522 <[email protected]>

fmt check

Signed-off-by: shruti2522 <[email protected]>

feat: added FunctionSymbol

Signed-off-by: shruti2522 <[email protected]>

fmt check

Signed-off-by: shruti2522 <[email protected]>

feat: added FunctionSymbol

Signed-off-by: shruti2522 <[email protected]>

add function symbol kind to lsp

Signed-off-by: shruti2522 <[email protected]>

update function symbol def

Signed-off-by: shruti2522 <[email protected]>

fmt check

Signed-off-by: shruti2522 <[email protected]>

fix ci

Signed-off-by: shruti2522 <[email protected]>

fix ci

Signed-off-by: shruti2522 <[email protected]>

feat: added FunctionSymbol

Signed-off-by: shruti2522 <[email protected]>

add tests to sema_token

Signed-off-by: shruti2522 <[email protected]>

add get_function_symbol

Signed-off-by: shruti2522 <[email protected]>

highlight for func call

Signed-off-by: shruti2522 <[email protected]>

fmt check

Signed-off-by: shruti2522 <[email protected]>

fmt check

Signed-off-by: shruti2522 <[email protected]>

modify walk_call_expr

Signed-off-by: shruti2522 <[email protected]>

update namer for func_name

Signed-off-by: shruti2522 <[email protected]>

feat: distinguish highlight for func symbol and normal var symbol

Signed-off-by: shruti2522 <[email protected]>

update alloc_function_symbol

Signed-off-by: shruti2522 <[email protected]>

function symbol for builtin functions

Signed-off-by: shruti2522 <[email protected]>

update function symbol def

Signed-off-by: shruti2522 <[email protected]>

delete test.log

Signed-off-by: shruti2522 <[email protected]>

remove func symbol from walk_call_expr

Signed-off-by: shruti2522 <[email protected]>

update walk_call_expr for namer

Signed-off-by: shruti2522 <[email protected]>

update sema test

Signed-off-by: shruti2522 <[email protected]>

feat: added FunctionSymbol

Signed-off-by: shruti2522 <[email protected]>

add function symbol kind in global state

Signed-off-by: shruti2522 <[email protected]>

update hover to handle func symbolkind

Signed-off-by: shruti2522 <[email protected]>

add condition for symbolkind in hover

Signed-off-by: shruti2522 <[email protected]>

fmt check

Signed-off-by: shruti2522 <[email protected]>

remove print statements

Signed-off-by: shruti2522 <[email protected]>

update loader snapshots

Signed-off-by: shruti2522 <[email protected]>

fix advanced resolver

Signed-off-by: shruti2522 <[email protected]>

feat: added FunctionSymbol

Signed-off-by: shruti2522 <[email protected]>

update assign stmt for lambda expr

Signed-off-by: shruti2522 <[email protected]>

update resolve_names for identifier_expr

Signed-off-by: shruti2522 <[email protected]>

update semantic tokens with function type

Signed-off-by: shruti2522 <[email protected]>

feat: api list_variables supports get variables from multi-files (kcl-lang#1389)

* feat: api list_variables supports get variables from multi-files

Signed-off-by: zongz <[email protected]>

* fix: fix CR comments

Signed-off-by: zongz <[email protected]>

* fix: fix test cases

Signed-off-by: zongz <[email protected]>

---------

Signed-off-by: zongz <[email protected]>

feat: advance resolver incremental compile (kcl-lang#1209)

feat: advanced resolver incremental compile. 1. Namer and Advanced Resolver support incremental compilation, clear cache by pkg and only update new pkg
2. Add gs cache in lsp state
3. Namer and Advanced Resolver modify gs in place(&mut) to reduce clone

Signed-off-by: he1pa <[email protected]>

feat: added FunctionSymbol

Signed-off-by: shruti2522 <[email protected]>

resolve conflicts

Signed-off-by: shruti2522 <[email protected]>

resolve conflicts

Signed-off-by: shruti2522 <[email protected]>

feat: added FunctionSymbol

Signed-off-by: shruti2522 <[email protected]>

resolve conflicts

Signed-off-by: shruti2522 <[email protected]>

remove duplicate code

Signed-off-by: shruti2522 <[email protected]>

fmt check

Signed-off-by: shruti2522 <[email protected]>

remove repititions

Signed-off-by: shruti2522 <[email protected]>

fmt check

Signed-off-by: shruti2522 <[email protected]>

feat: added FunctionSymbol

Signed-off-by: shruti2522 <[email protected]>
shruti2522 and others added 3 commits June 3, 2024 19:52
Signed-off-by: shruti2522 <[email protected]>

remove unwanted changes in global_state

Signed-off-by: shruti2522 <[email protected]>
Signed-off-by: shruti2522 <[email protected]>
Copy link
Contributor

@Peefy Peefy left a comment

Choose a reason for hiding this comment

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

LGTM

@Peefy Peefy merged commit 3e40c8f into kcl-lang:main Jun 3, 2024
10 of 11 checks passed
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.

[Enhancement] Distinguish the highlight of function symbol and normal var symbol
4 participants