From 11f2b48853ef244ee1aa114cae5c7e738b11d24c Mon Sep 17 00:00:00 2001 From: Shunsuke Shibayama Date: Mon, 25 Mar 2024 00:13:03 +0900 Subject: [PATCH] feat: add `Linter` --- Cargo.lock | 10 + Cargo.toml | 15 +- crates/els/diagnostics.rs | 1 - crates/erg_common/config.rs | 3 + crates/erg_compiler/error/mod.rs | 8 + crates/erg_linter/Cargo.toml | 49 ++++ crates/erg_linter/README.md | 8 +- crates/erg_linter/{mod.rs => lib.rs} | 1 + crates/erg_linter/lint.rs | 322 ++++++++++++++++++++++++++- crates/erg_linter/warn.rs | 17 ++ src/dummy.rs | 2 +- src/main.rs | 2 + tests/common.rs | 2 +- 13 files changed, 422 insertions(+), 18 deletions(-) create mode 100644 crates/erg_linter/Cargo.toml rename crates/erg_linter/{mod.rs => lib.rs} (76%) create mode 100644 crates/erg_linter/warn.rs diff --git a/Cargo.lock b/Cargo.lock index 10e27d8ba..d3ecbdfaa 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -112,6 +112,7 @@ dependencies = [ "els", "erg_common", "erg_compiler", + "erg_linter", "erg_parser", ] @@ -136,6 +137,15 @@ dependencies = [ "pyo3", ] +[[package]] +name = "erg_linter" +version = "0.6.33" +dependencies = [ + "erg_common", + "erg_compiler", + "erg_parser", +] + [[package]] name = "erg_parser" version = "0.6.33" diff --git a/Cargo.toml b/Cargo.toml index 67d9b94a0..c8cb676e8 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -15,6 +15,7 @@ homepage.workspace = true members = [ "crates/erg_common", "crates/erg_compiler", + "crates/erg_linter/", "crates/erg_parser", "crates/els", "crates/erg_proc_macros/", @@ -30,45 +31,50 @@ homepage = "https://erg-lang.org/" [features] # when "debug" feature is turned on, that of the following crates will also be turned on. -debug = ["erg_common/debug", "erg_parser/debug", "erg_compiler/debug"] # "els/debug" +debug = ["erg_common/debug", "erg_parser/debug", "erg_compiler/debug", "erg_linter/debug"] # "els/debug" backtrace = ["erg_common/backtrace", "els/backtrace"] japanese = [ "erg_common/japanese", "erg_parser/japanese", "erg_compiler/japanese", "els/japanese", + "erg_linter/japanese", ] simplified_chinese = [ "erg_common/simplified_chinese", "erg_parser/simplified_chinese", "erg_compiler/simplified_chinese", "els/simplified_chinese", + "erg_linter/simplified_chinese", ] traditional_chinese = [ "erg_common/traditional_chinese", "erg_parser/traditional_chinese", "erg_compiler/traditional_chinese", "els/traditional_chinese", + "erg_linter/traditional_chinese", ] -unicode = ["erg_common/unicode", "erg_parser/unicode", "erg_compiler/unicode", "els/unicode"] -pretty = ["erg_common/pretty", "erg_parser/pretty", "erg_compiler/pretty", "els/pretty"] +unicode = ["erg_common/unicode", "erg_parser/unicode", "erg_compiler/unicode", "els/unicode", "erg_linter/unicode"] +pretty = ["erg_common/pretty", "erg_parser/pretty", "erg_compiler/pretty", "els/pretty", "erg_linter/pretty"] large_thread = [ "erg_common/large_thread", "erg_parser/large_thread", "erg_compiler/large_thread", "els/large_thread", + "erg_linter/large_thread", ] py_compat = ["erg_compiler/py_compat", "els/py_compat"] gal = ["erg_common/gal", "erg_compiler/gal"] els = ["erg_common/els", "erg_compiler/els", "dep:els"] full-repl = ["erg_common/full-repl"] full = ["els", "full-repl", "unicode", "pretty"] -experimental = ["erg_common/experimental", "erg_parser/experimental", "erg_compiler/experimental"] +experimental = ["erg_common/experimental", "erg_parser/experimental", "erg_compiler/experimental", "erg_linter/experimental"] [workspace.dependencies] erg_common = { version = "0.6.33", path = "./crates/erg_common" } erg_parser = { version = "0.6.33", path = "./crates/erg_parser" } erg_compiler = { version = "0.6.33", path = "./crates/erg_compiler" } +erg_linter = { version = "0.6.33", path = "./crates/erg_linter" } els = { version = "0.1.45", path = "./crates/els" } erg_proc_macros = { version = "0.6.33", path = "./crates/erg_proc_macros" } pyo3 = { version = "0.20", features = ["extension-module"] } @@ -77,6 +83,7 @@ pyo3 = { version = "0.20", features = ["extension-module"] } erg_common = { workspace = true } erg_parser = { workspace = true } erg_compiler = { workspace = true } +erg_linter = { workspace = true } els = { workspace = true, optional = true } [build-dependencies] diff --git a/crates/els/diagnostics.rs b/crates/els/diagnostics.rs index 430d16289..36fe31902 100644 --- a/crates/els/diagnostics.rs +++ b/crates/els/diagnostics.rs @@ -11,7 +11,6 @@ use erg_common::dict::Dict; use erg_common::pathutil::{project_entry_file_of, project_root_dir_of}; use erg_common::spawn::{safe_yield, spawn_new_thread}; use erg_common::style::*; -use erg_common::traits::Stream; use erg_common::{fn_name, lsp_log}; use erg_compiler::artifact::BuildRunnable; use erg_compiler::build_package::CheckStatus; diff --git a/crates/erg_common/config.rs b/crates/erg_common/config.rs index a0e570300..6ad746a8a 100644 --- a/crates/erg_common/config.rs +++ b/crates/erg_common/config.rs @@ -31,6 +31,7 @@ pub enum ErgMode { Transpile, Execute, LanguageServer, + Lint, Read, Pack, } @@ -48,6 +49,7 @@ impl TryFrom<&str> for ErgMode { "trans" | "transpile" | "transpiler" => Ok(Self::Transpile), "run" | "execute" => Ok(Self::Execute), "server" | "language-server" => Ok(Self::LanguageServer), + "lint" | "linter" => Ok(Self::Lint), "byteread" | "read" | "reader" | "dis" => Ok(Self::Read), "pack" | "package" => Ok(Self::Pack), _ => Err(()), @@ -67,6 +69,7 @@ impl From for &str { ErgMode::Transpile => "transpile", ErgMode::Execute => "execute", ErgMode::LanguageServer => "language-server", + ErgMode::Lint => "lint", ErgMode::Read => "read", ErgMode::Pack => "pack", } diff --git a/crates/erg_compiler/error/mod.rs b/crates/erg_compiler/error/mod.rs index a4d060160..351e6dd40 100644 --- a/crates/erg_compiler/error/mod.rs +++ b/crates/erg_compiler/error/mod.rs @@ -646,6 +646,14 @@ impl CompileErrors { pub fn take(&mut self) -> Self { self.flush() } + + pub fn len(&self) -> usize { + self.0.len() + } + + pub fn is_empty(&self) -> bool { + self.0.is_empty() + } } pub type SingleCompileResult = Result; diff --git a/crates/erg_linter/Cargo.toml b/crates/erg_linter/Cargo.toml new file mode 100644 index 000000000..363857516 --- /dev/null +++ b/crates/erg_linter/Cargo.toml @@ -0,0 +1,49 @@ +[package] +name = "erg_linter" +description = "the Erg linter" +documentation = "http://docs.rs/erg_linter" +version.workspace = true +authors.workspace = true +license.workspace = true +edition.workspace = true +repository.workspace = true +homepage.workspace = true + +[features] +debug = ["erg_common/debug", "erg_parser/debug", "erg_compiler/debug"] +backtrace = ["erg_common/backtrace"] +japanese = [ + "erg_common/japanese", + "erg_parser/japanese", + "erg_compiler/japanese", +] +simplified_chinese = [ + "erg_common/simplified_chinese", + "erg_parser/simplified_chinese", + "erg_compiler/simplified_chinese", +] +traditional_chinese = [ + "erg_common/traditional_chinese", + "erg_parser/traditional_chinese", + "erg_compiler/traditional_chinese", +] +unicode = ["erg_common/unicode", "erg_parser/unicode", "erg_compiler/unicode"] +pretty = ["erg_common/pretty", "erg_parser/pretty", "erg_compiler/pretty"] +large_thread = [ + "erg_common/large_thread", + "erg_parser/large_thread", + "erg_compiler/large_thread", +] +py_compat = ["erg_compiler/py_compat"] +gal = ["erg_common/gal", "erg_compiler/gal"] +full-repl = ["erg_common/full-repl"] +full = ["full-repl", "unicode", "pretty"] +experimental = ["erg_common/experimental", "erg_parser/experimental", "erg_compiler/experimental"] + +[dependencies] +erg_common = { workspace = true } +erg_parser = { workspace = true } +erg_compiler ={ workspace = true } + +[lib] +path = "lib.rs" diff --git a/crates/erg_linter/README.md b/crates/erg_linter/README.md index 9e77b546c..9b729b9c1 100644 --- a/crates/erg_linter/README.md +++ b/crates/erg_linter/README.md @@ -8,12 +8,16 @@ The following codes are warned. * Unreachable codes * Wildcard import -* Unused variables * Shadowing of built-in variables -* Unused objects that are not `NoneLike` * Procedures without side-effects * Variables that can be defined as constants * Unnecessary `.clone` * Mutable objects that do not change * Hardcoded well-known constants (e.g. `3.14`) * Defining a subroutine with too many parameters +* Defining a class with too many fields + +### These are warned by the compiler + +* Unused variables +* Unused objects that are not `NoneLike` diff --git a/crates/erg_linter/mod.rs b/crates/erg_linter/lib.rs similarity index 76% rename from crates/erg_linter/mod.rs rename to crates/erg_linter/lib.rs index eb6229e18..7603f0ea9 100644 --- a/crates/erg_linter/mod.rs +++ b/crates/erg_linter/lib.rs @@ -1,3 +1,4 @@ mod lint; +mod warn; pub use lint::Linter; diff --git a/crates/erg_linter/lint.rs b/crates/erg_linter/lint.rs index da21604e9..21086a3f7 100644 --- a/crates/erg_linter/lint.rs +++ b/crates/erg_linter/lint.rs @@ -1,19 +1,323 @@ -use erg_common::Str; +use erg_common::config::ErgConfig; +use erg_common::error::ErrorKind; +use erg_common::error::{ErrorDisplay, MultiErrorDisplay}; +use erg_common::io::Input; +use erg_common::log; +use erg_common::traits::{BlockKind, ExitStatus, Locational, New, Runnable, Stream}; -use crate::error::CompileWarnings; -use crate::hir::HIR; +use erg_compiler::artifact::{Buildable, ErrorArtifact}; +use erg_compiler::build_package::PackageBuilder; +use erg_compiler::error::{CompileError, CompileErrors, CompileWarnings}; +use erg_compiler::hir::{Accessor, Array, Def, Dict, Expr, Set, Signature, Tuple}; +use erg_compiler::module::SharedCompilerResource; -#[derive(Debug, Default)] +use erg_parser::ParserRunner; + +use crate::warn::too_many_params; + +#[derive(Debug)] pub struct Linter { - _used: Vec, + pub cfg: ErgConfig, + builder: PackageBuilder, + warns: CompileWarnings, +} + +impl Default for Linter { + fn default() -> Self { + Self::new(ErgConfig::default()) + } +} + +impl New for Linter { + fn new(cfg: ErgConfig) -> Self { + let shared = SharedCompilerResource::new(cfg.copy()); + Self { + builder: PackageBuilder::new(cfg.copy(), shared), + cfg, + warns: CompileWarnings::empty(), + } + } +} + +impl Runnable for Linter { + type Err = CompileError; + type Errs = CompileErrors; + const NAME: &'static str = "Erg linter"; + + #[inline] + fn cfg(&self) -> &ErgConfig { + &self.cfg + } + #[inline] + fn cfg_mut(&mut self) -> &mut ErgConfig { + &mut self.cfg + } + + #[inline] + fn finish(&mut self) {} + + fn initialize(&mut self) { + self.builder.initialize(); + self.warns.clear(); + } + + fn clear(&mut self) { + self.builder.clear(); + self.warns.clear(); + } + + fn set_input(&mut self, input: erg_common::io::Input) { + self.cfg.input = input; + self.builder.set_input(self.cfg.input.clone()); + } + + fn exec(&mut self) -> Result { + let warns = self.lint_module().map_err(|eart| { + eart.warns.write_all_stderr(); + eart.errors + })?; + warns.write_all_stderr(); + Ok(ExitStatus::compile_passed(warns.len())) + } + + fn eval(&mut self, src: String) -> Result { + let warns = self.lint(src).map_err(|eart| { + eart.warns.write_all_stderr(); + eart.errors + })?; + warns.write_all_stderr(); + Ok("OK".to_string()) + } + + fn expect_block(&self, src: &str) -> BlockKind { + let mut parser = ParserRunner::new(self.cfg().clone()); + match parser.eval(src.to_string()) { + Err(errs) => { + let kind = errs + .iter() + .filter(|e| e.core().kind == ErrorKind::ExpectNextLine) + .map(|e| { + let msg = e.core().sub_messages.last().unwrap(); + // ExpectNextLine error must have msg otherwise it's a bug + msg.get_msg().first().unwrap().to_owned() + }) + .next(); + if let Some(kind) = kind { + return BlockKind::from(kind.as_str()); + } + if errs + .iter() + .any(|err| err.core.main_message.contains("\"\"\"")) + { + return BlockKind::MultiLineStr; + } + BlockKind::Error + } + Ok(_) => { + if src.contains("Class") { + return BlockKind::ClassDef; + } + BlockKind::None + } + } + } } impl Linter { - pub fn new() -> Self { - Self { _used: Vec::new() } + pub fn new(cfg: ErgConfig) -> Self { + New::new(cfg) + } + + fn caused_by(&self) -> String { + self.builder.get_context().unwrap().context.caused_by() + } + + fn input(&self) -> Input { + self.builder.input().clone() + } + + pub fn lint_module(&mut self) -> Result { + let src = self.input().read(); + self.lint(src) + } + + pub fn lint(&mut self, src: String) -> Result { + log!(info "Start linting"); + let art = self.builder.build(src, "exec")?; + self.warns.extend(art.warns); + for chunk in art.object.module.iter() { + self.lint_too_many_params(chunk); + } + log!(info "Finished linting"); + Ok(self.warns.take()) + } + + fn lint_too_many_params(&mut self, chunk: &Expr) { + match chunk { + Expr::Def(Def { + sig: Signature::Subr(subr), + body, + }) => { + if subr.params.len() >= 8 { + self.warns.push(too_many_params( + self.input(), + self.caused_by(), + subr.params.loc(), + )); + } + for chunk in body.block.iter() { + self.lint_too_many_params(chunk); + } + } + _ => self.check_recursively(&Self::lint_too_many_params, chunk), + } + } + + /* ↓ Helper methods ↓ */ + fn check_recursively(&mut self, lint_fn: &impl Fn(&mut Linter, &Expr), expr: &Expr) { + match expr { + Expr::Literal(_) => {} + Expr::Record(record) => { + for attr in record.attrs.iter() { + for chunk in attr.body.block.iter() { + lint_fn(self, chunk); + } + if let Signature::Subr(subr) = &attr.sig { + for param in subr.params.defaults.iter() { + lint_fn(self, ¶m.default_val); + } + } + } + } + Expr::Array(array) => match array { + Array::Normal(arr) => { + for elem in arr.elems.pos_args.iter() { + lint_fn(self, &elem.expr); + } + } + Array::WithLength(arr) => { + lint_fn(self, &arr.elem); + if let Some(len) = &arr.len { + lint_fn(self, len); + } + } + Array::Comprehension(arr) => { + lint_fn(self, &arr.elem); + lint_fn(self, &arr.guard); + } + }, + Expr::Tuple(tuple) => match tuple { + Tuple::Normal(tup) => { + for elem in tup.elems.pos_args.iter() { + lint_fn(self, &elem.expr); + } + } + }, + Expr::Set(set) => match set { + Set::Normal(set) => { + for elem in set.elems.pos_args.iter() { + lint_fn(self, &elem.expr); + } + } + Set::WithLength(set) => { + lint_fn(self, &set.elem); + lint_fn(self, &set.len); + } + }, + Expr::Dict(dict) => match dict { + Dict::Normal(dic) => { + for kv in dic.kvs.iter() { + lint_fn(self, &kv.key); + lint_fn(self, &kv.value); + } + } + _ => { + log!("Dict comprehension not implemented"); + } + }, + Expr::BinOp(binop) => { + lint_fn(self, &binop.lhs); + lint_fn(self, &binop.rhs); + } + Expr::UnaryOp(unaryop) => { + lint_fn(self, &unaryop.expr); + } + Expr::Call(call) => { + lint_fn(self, &call.obj); + for arg in call.args.iter() { + lint_fn(self, arg); + } + } + Expr::Def(def) => { + for chunk in def.body.block.iter() { + lint_fn(self, chunk); + } + if let Signature::Subr(subr) = &def.sig { + for param in subr.params.defaults.iter() { + lint_fn(self, ¶m.default_val); + } + } + } + Expr::ClassDef(class_def) => { + if let Signature::Subr(subr) = &class_def.sig { + for param in subr.params.defaults.iter() { + lint_fn(self, ¶m.default_val); + } + } + for methods in class_def.methods_list.iter() { + for method in methods.defs.iter() { + lint_fn(self, method); + } + } + } + Expr::PatchDef(class_def) => { + if let Signature::Subr(subr) = &class_def.sig { + for param in subr.params.defaults.iter() { + lint_fn(self, ¶m.default_val); + } + } + for method in class_def.methods.iter() { + lint_fn(self, method); + } + } + Expr::ReDef(redef) => { + self.check_acc_recursively(lint_fn, &redef.attr); + for chunk in redef.block.iter() { + lint_fn(self, chunk); + } + } + Expr::Lambda(lambda) => { + for chunk in lambda.body.iter() { + lint_fn(self, chunk); + } + for param in lambda.params.defaults.iter() { + lint_fn(self, ¶m.default_val); + } + } + Expr::TypeAsc(tasc) => { + lint_fn(self, &tasc.expr); + } + Expr::Accessor(acc) => { + self.check_acc_recursively(lint_fn, acc); + } + Expr::Compound(exprs) => { + for chunk in exprs.iter() { + lint_fn(self, chunk); + } + } + Expr::Dummy(exprs) => { + for chunk in exprs.iter() { + lint_fn(self, chunk); + } + } + _ => {} + } } - pub fn lint(&mut self, _hir: &HIR) -> CompileWarnings { - CompileWarnings::empty() + fn check_acc_recursively(&mut self, lint_fn: impl Fn(&mut Linter, &Expr), acc: &Accessor) { + match acc { + Accessor::Attr(attr) => lint_fn(self, &attr.obj), + Accessor::Ident(_) => {} + } } } diff --git a/crates/erg_linter/warn.rs b/crates/erg_linter/warn.rs new file mode 100644 index 000000000..4228bfb0f --- /dev/null +++ b/crates/erg_linter/warn.rs @@ -0,0 +1,17 @@ +use erg_common::error::{ErrorCore, ErrorKind, Location}; +use erg_common::io::Input; +use erg_compiler::error::CompileWarning; + +pub(crate) fn too_many_params(input: Input, caused_by: String, loc: Location) -> CompileWarning { + CompileWarning::new( + ErrorCore::new( + vec![], + "too many parameters".to_string(), + 0, + ErrorKind::Warning, + loc, + ), + input, + caused_by, + ) +} diff --git a/src/dummy.rs b/src/dummy.rs index 7b44d38e2..cb65d8e8d 100644 --- a/src/dummy.rs +++ b/src/dummy.rs @@ -8,7 +8,7 @@ use std::time::Duration; use erg_common::config::ErgConfig; use erg_common::error::{ErrorDisplay, ErrorKind, MultiErrorDisplay}; use erg_common::python_util::spawn_py; -use erg_common::traits::{BlockKind, ExitStatus, New, Runnable, Stream}; +use erg_common::traits::{BlockKind, ExitStatus, New, Runnable}; use erg_compiler::hir::Expr; use erg_compiler::ty::HasType; diff --git a/src/main.rs b/src/main.rs index 0b2397fad..5a1c10138 100644 --- a/src/main.rs +++ b/src/main.rs @@ -7,6 +7,7 @@ use erg_common::spawn::exec_new_thread; use erg_common::traits::{ExitStatus, Runnable}; use erg_compiler::build_package::{PackageBuilder, PackageTypeChecker}; +use erg_linter::Linter; use erg_parser::lex::LexerRunner; use erg_parser::ParserRunner; @@ -29,6 +30,7 @@ fn run() { Execute => DummyVM::run(cfg), Read => Deserializer::run(cfg), Pack => PackageManagerRunner::run(cfg), + Lint => Linter::run(cfg), LanguageServer => { #[cfg(feature = "els")] { diff --git a/tests/common.rs b/tests/common.rs index 32554f785..d1207c37a 100644 --- a/tests/common.rs +++ b/tests/common.rs @@ -9,7 +9,7 @@ use erg_common::python_util::PythonVersion; use erg_common::spawn::exec_new_thread; use erg_common::style::remove_style; use erg_common::style::{colors::DEBUG_MAIN, RESET}; -use erg_common::traits::{ExitStatus, Runnable, Stream}; +use erg_common::traits::{ExitStatus, Runnable}; use erg_compiler::error::CompileErrors; use erg_compiler::Compiler;