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.

Sign up for GitHub

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

Jump to bottom

feat: lint check unused import in each file instead of global modules #724

Merged
merged 2 commits into from
Sep 20, 2023
Merged

feat: lint check unused import in each file instead of global modules #724

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
72c040c
feat: lint check unused import in each file instead of global modules
He1pa Sep 20, 2023
ff09275
test: fix ut
He1pa Sep 20, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions kclvm/sema/src/lint/lints_def.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,8 +100,8 @@ impl LintPass for UnusedImport {
for (_, scope_obj) in scope_objs {
let scope_obj = scope_obj.borrow();
if let ScopeObjectKind::Module(m) = &scope_obj.kind {
if !scope_obj.used {
for stmt in &m.import_stmts {
for (stmt, has_used) in &m.import_stmts {
if !has_used {
handler.add_warning(
WarningKind::UnusedImportWarning,
&[Message {
Expand Down
3 changes: 0 additions & 3 deletions kclvm/sema/src/resolver/config.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,6 @@ impl<'ctx> Resolver<'ctx> {
end,
ty,
kind: ScopeObjectKind::Attribute,
used: false,
doc: None,
}
}
Expand Down Expand Up @@ -440,7 +439,6 @@ impl<'ctx> Resolver<'ctx> {
end: key.get_end_pos(),
ty: val_ty.clone(),
kind: ScopeObjectKind::Attribute,
used: false,
doc: None,
},
);
Expand Down Expand Up @@ -474,7 +472,6 @@ impl<'ctx> Resolver<'ctx> {
end: key.get_end_pos(),
ty: val_ty.clone(),
kind: ScopeObjectKind::Attribute,
used: false,
doc: None,
},
);
Expand Down
4 changes: 0 additions & 4 deletions kclvm/sema/src/resolver/global.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,6 @@ impl<'ctx> Resolver<'ctx> {
end,
ty: Rc::new(Type::schema(schema_ty.clone())),
kind: ScopeObjectKind::Definition,
used: false,
doc: Some(parsed_doc.summary.clone()),
},
)
Expand Down Expand Up @@ -141,7 +140,6 @@ impl<'ctx> Resolver<'ctx> {
end,
ty: Rc::new(Type::schema(schema_ty.clone())),
kind: ScopeObjectKind::Definition,
used: false,
doc: Some(schema_ty.doc),
},
)
Expand Down Expand Up @@ -303,7 +301,6 @@ impl<'ctx> Resolver<'ctx> {
end,
ty,
kind: ScopeObjectKind::Variable,
used: false,
doc: None,
},
);
Expand Down Expand Up @@ -363,7 +360,6 @@ impl<'ctx> Resolver<'ctx> {
end,
ty,
kind: ScopeObjectKind::Variable,
used: false,
doc: None,
},
);
Expand Down
8 changes: 3 additions & 5 deletions kclvm/sema/src/resolver/import.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ impl<'ctx> Resolver<'ctx> {
let mut obj = scope_obj.borrow_mut();
match &mut obj.kind {
ScopeObjectKind::Module(m) => {
m.import_stmts.push(stmt.clone())
m.import_stmts.push((stmt.clone(), false))
},
_ => bug!(
"invalid module type in the import check function {}",
Expand Down Expand Up @@ -170,9 +170,8 @@ impl<'ctx> Resolver<'ctx> {
end: end.clone(),
ty: Rc::new(ty.clone()),
kind: ScopeObjectKind::Module(Module {
import_stmts: vec![stmt.clone()],
import_stmts: vec![(stmt.clone(), true)],
}),
used: true,
doc: None,
})),
);
Expand All @@ -184,9 +183,8 @@ impl<'ctx> Resolver<'ctx> {
end,
ty: Rc::new(ty),
kind: ScopeObjectKind::Module(Module {
import_stmts: vec![stmt.clone()],
import_stmts: vec![(stmt.clone(), false)],
}),
used: false,
doc: None,
})),
);
Expand Down
6 changes: 0 additions & 6 deletions kclvm/sema/src/resolver/node.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,6 @@ impl<'ctx> MutSelfTypedResultWalker<'ctx> for Resolver<'ctx> {
end,
ty: ty.clone(),
kind: ScopeObjectKind::TypeAlias,
used: false,
doc: None,
},
);
Expand Down Expand Up @@ -328,7 +327,6 @@ impl<'ctx> MutSelfTypedResultWalker<'ctx> for Resolver<'ctx> {
end,
ty: self.any_ty(),
kind: ScopeObjectKind::Variable,
used: false,
doc: None,
},
);
Expand Down Expand Up @@ -383,7 +381,6 @@ impl<'ctx> MutSelfTypedResultWalker<'ctx> for Resolver<'ctx> {
end,
ty: expected_ty.clone(),
kind: ScopeObjectKind::Attribute,
used: false,
doc: doc_str,
},
);
Expand Down Expand Up @@ -516,7 +513,6 @@ impl<'ctx> MutSelfTypedResultWalker<'ctx> for Resolver<'ctx> {
end: selector_expr.attr.get_end_pos(),
ty: value_ty.clone(),
kind: ScopeObjectKind::FunctionCall,
used: false,
doc: Some(func.doc.clone()),
},
)
Expand Down Expand Up @@ -819,7 +815,6 @@ impl<'ctx> MutSelfTypedResultWalker<'ctx> for Resolver<'ctx> {
end,
ty: self.any_ty(),
kind: ScopeObjectKind::Variable,
used: false,
doc: None,
},
);
Expand Down Expand Up @@ -969,7 +964,6 @@ impl<'ctx> MutSelfTypedResultWalker<'ctx> for Resolver<'ctx> {
end: end.clone(),
ty: param.ty.clone(),
kind: ScopeObjectKind::Parameter,
used: false,
doc: None,
},
)
Expand Down
4 changes: 0 additions & 4 deletions kclvm/sema/src/resolver/schema.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,6 @@ impl<'ctx> Resolver<'ctx> {
end: end.clone(),
ty: param.ty.clone(),
kind: ScopeObjectKind::Parameter,
used: false,
doc: None,
},
)
Expand All @@ -72,7 +71,6 @@ impl<'ctx> Resolver<'ctx> {
end,
ty: index_signature.key_ty.clone(),
kind: ScopeObjectKind::Variable,
used: false,
doc: None,
},
)
Expand All @@ -93,7 +91,6 @@ impl<'ctx> Resolver<'ctx> {
end: Position::dummy_pos(),
ty: self.any_ty(),
kind: ScopeObjectKind::Variable,
used: false,
doc: None,
},
);
Expand Down Expand Up @@ -144,7 +141,6 @@ impl<'ctx> Resolver<'ctx> {
end: end.clone(),
ty: param.ty.clone(),
kind: ScopeObjectKind::Parameter,
used: false,
doc: None,
},
)
Expand Down
8 changes: 3 additions & 5 deletions kclvm/sema/src/resolver/scope.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,6 @@ pub struct ScopeObject {
pub ty: Rc<Type>,
/// The scope object kind.
pub kind: ScopeObjectKind,
/// Record whether has been used, for check unused imported module and var definition
pub used: bool,
/// The doc of the scope object, will be None unless the scope object represents a schema or schema attribute.
pub doc: Option<String>,
}
Expand Down Expand Up @@ -81,7 +79,8 @@ pub enum ScopeObjectKind {
/// is used to record information on the AST
#[derive(PartialEq, Clone, Debug)]
pub struct Module {
pub import_stmts: Vec<NodeRef<Stmt>>,
/// Record stmts which import this module and whether has been used, for check unused imported module and var definition
pub import_stmts: Vec<(NodeRef<Stmt>, bool)>,
}

/// A Scope maintains a set of objects and links to its containing
Expand Down Expand Up @@ -136,7 +135,7 @@ impl Scope {
match &obj.borrow().kind {
ScopeObjectKind::Module(module) => {
for stmt in &module.import_stmts {
if let Import(import_stmt) = &stmt.node {
if let Import(import_stmt) = &stmt.0.node {
res.insert(import_stmt.name.clone(), obj.clone());
}
}
Expand Down Expand Up @@ -345,7 +344,6 @@ pub(crate) fn builtin_scope() -> Scope {
end: Position::dummy_pos(),
ty: Rc::new(builtin_func.clone()),
kind: ScopeObjectKind::Definition,
used: false,
doc: None,
})),
);
Expand Down
11 changes: 8 additions & 3 deletions kclvm/sema/src/resolver/var.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
use crate::resolver::Resolver;
use crate::ty::TypeKind;
use indexmap::IndexMap;
use kclvm_ast::pos::GetPos;
use kclvm_error::diagnostic::Range;
use kclvm_error::*;

Expand Down Expand Up @@ -45,7 +46,6 @@ impl<'ctx> Resolver<'ctx> {
end: range.1.clone(),
ty: self.any_ty(),
kind: ScopeObjectKind::Variable,
used: false,
doc: None,
},
);
Expand Down Expand Up @@ -73,7 +73,6 @@ impl<'ctx> Resolver<'ctx> {
end: range.1.clone(),
ty: self.any_ty(),
kind: ScopeObjectKind::Variable,
used: false,
doc: None,
},
);
Expand All @@ -87,7 +86,13 @@ impl<'ctx> Resolver<'ctx> {
// It should be recursively search whole scope to lookup scope object, not the current scope.element.
if !pkgpath.is_empty() {
if let Some(obj) = self.scope.borrow().lookup(pkgpath) {
obj.borrow_mut().used = true;
if let ScopeObjectKind::Module(m) = &mut obj.borrow_mut().kind {
for (stmt, used) in m.import_stmts.iter_mut() {
if stmt.get_pos().filename == range.0.filename {
*used = true;
}
}
}
}
}
// Load type
Expand Down
2 changes: 1 addition & 1 deletion kclvm/tools/src/LSP/src/completion.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ fn completion_variable(pos: &KCLPos, prog_scope: &ProgramScope) -> IndexSet<KCLC
match &obj.borrow().kind {
kclvm_sema::resolver::scope::ScopeObjectKind::Module(module) => {
for stmt in &module.import_stmts {
match &stmt.node {
match &stmt.0.node {
Stmt::Import(import_stmtt) => {
completions.insert(KCLCompletionItem {
label: import_stmtt.name.clone(),
Expand Down
1 change: 0 additions & 1 deletion kclvm/tools/src/LSP/src/goto_def.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -240,7 +240,6 @@ pub(crate) fn resolve_var(
end: func_name_node.get_end_pos(),
ty: ty.clone(),
kind: ScopeObjectKind::FunctionCall,
used: false,
doc: Some(func_ty.doc.clone()),
}))
}
Expand Down
1 change: 1 addition & 0 deletions kclvm/tools/src/lint/test_data/unused_check_for_each_file/a.k
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
import math
2 changes: 2 additions & 0 deletions kclvm/tools/src/lint/test_data/unused_check_for_each_file/b.k
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
import math
a = math.pow(1, 1)
27 changes: 27 additions & 0 deletions kclvm/tools/src/lint/tests.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,3 +31,30 @@ fn test_lint() {
assert_eq!(diag.messages[0].message, m.to_string());
}
}

#[test]
fn test_unused_check_for_each_file() {
let (errs, warnings) = lint_files(
&[
"./src/lint/test_data/unused_check_for_each_file/a.k",
"./src/lint/test_data/unused_check_for_each_file/b.k",
],
None,
);
assert_eq!(errs.len(), 0);
assert_eq!(warnings.len(), 1);
assert_eq!(
warnings[0].messages[0].message,
"Module 'math' imported but unused".to_string()
);
let mut path = PathBuf::from(env!("CARGO_MANIFEST_DIR"));
path.push("src");
path.push("lint");
path.push("test_data");
path.push("unused_check_for_each_file");
path.push("a.k");
assert_eq!(
warnings[0].messages[0].range.0.filename,
path.to_str().unwrap().to_string()
);
}
Loading