kcl-lang / kcl

KCL Programming Language (CNCF Sandbox Project). https://kcl-lang.io
https://kcl-lang.io
Apache License 2.0
1.42k stars 110 forks source link

feat: distinguish highlight for function symbol and normal var symbol #1386

Closed shruti2522 closed 1 month ago

shruti2522 commented 1 month ago

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):

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

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

coveralls commented 1 month ago

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


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%
<!-- Total: 121 196 61.73% -->
Files with Coverage Reduction New Missed Lines %
kclvm/tools/src/LSP/src/semantic_token.rs 1 90.38%
<!-- Total: 1 -->
Totals Coverage Status
Change from base Build 9349800499: -0.02%
Covered Lines: 54991
Relevant Lines: 77358

💛 - Coveralls
shruti2522 commented 1 month ago

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

Can you please review the changes, @Peefy

Peefy commented 1 month ago
  • 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 commented 1 month ago
  • 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 https://github.com/kcl-lang/kcl/blob/c07925fcd68a1690110ba9053a9b71106c3e52d2/kclvm/sema/src/namer/node.rs#L99

and walk_identifier_expr() in advanced_resolver https://github.com/kcl-lang/kcl/blob/c07925fcd68a1690110ba9053a9b71106c3e52d2/kclvm/sema/src/advanced_resolver/node.rs#L1074

shruti2522 commented 1 month ago

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 commented 1 month ago

@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 commented 1 month ago

PR conflicted.