-
Notifications
You must be signed in to change notification settings - Fork 126
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
[WIP] Refactor: Lint tool (Rust ver.) #103
Conversation
Pls benchmark resolver's core function. |
Pull Request Test Coverage Report for Build 2867182406
💛 - Coveralls |
02a0b71
to
cf76721
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pls add more unit tests in kclvm_ast
and kclvm_error
crate.
2b4eb8e
to
89621fe
Compare
89621fe
to
f6b5b71
Compare
ec7c365
to
371b1eb
Compare
04094d6
to
944f143
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Improvement suggestions implemented by lint.rs
, due to the lack of symbol table information in LintContext
, functions such as check_identifier
, check_schema_attr
and record_use
are implemented in a tricky way, which is part of the symbol table implementation (we can query the variable type and whether the variable has been used. )
ba6160b
to
c258c2f
Compare
f75c767
to
6a8ee73
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remove samples/a.json
78fee86
to
568be41
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Important things:
- move
lint.rs
andlint_def.rs
to thelint module
- re-org the lint module
- lint.rs
- lint_def.rs
- macros.rs
819a931
to
f76f8a5
Compare
1. Add warning level diagnotics, including 'UnusedImportWarning', 'ReimportWarning' and 'ImportPostionWarning' 2. Add 'lint_check_module' method to Resolver and is called in the 'check()' method. 3. Add KCL Lint stuct, For details see issue kcl-lang#109 fix kcl-lang#102, kcl-lang#109
f76f8a5
to
ea28cc7
Compare
This pr is being split into multiple sub-pr and will be closed when complete. |
Split large WIP PRs into smaller ones |
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/kclvm/sema/src/resolver
KCLVM/kclvm/error
KCLVM/kclvm/ast/walker.rs
3. Provide a description of the PR(e.g. more details, effects, motivations or doc link):
Add lint check in resolver.
Now, these diags are only stored in resolver.handler.diagonstics and are not thrown to the user.
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:
KCLVM/kclvm/sema/src/resolver/test.rs::test_lint()
6. Release note
Please refer to Release Notes Language Style Guide to write a quality release note.
7. Benchmark
Before
After
Before
After