Skip to content

Commit

Permalink
feat: auto fix tools (#719)
Browse files Browse the repository at this point in the history
* feat: auto fix tools. Add '--fix' param for kclvm_cli lint and auto fix unused import and reimport warning

* add license file

* test: add ut for fix tool

* fix: fix newline character different in windows
  • Loading branch information
He1pa authored Sep 22, 2023
1 parent 44d81f4 commit eeff00e
Show file tree
Hide file tree
Showing 25 changed files with 767 additions and 6 deletions.
3 changes: 2 additions & 1 deletion kclvm/cmd/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,8 @@ pub fn app() -> Command {
.arg(arg!(path_selector: -S --path_selector <path_selector> ... "Specify the path selector").num_args(1..))
.arg(arg!(overrides: -O --overrides <overrides> ... "Specify the configuration override path and value").num_args(1..))
.arg(arg!(target: --target <target> "Specify the target type"))
.arg(arg!(package_map: -E --external <package_map> ... "Mapping of package name and path where the package is located").num_args(1..)),
.arg(arg!(package_map: -E --external <package_map> ... "Mapping of package name and path where the package is located").num_args(1..))
.arg(arg!(fix: -f --fix "Auto fix")),
)
.subcommand(
Command::new("fmt")
Expand Down
10 changes: 9 additions & 1 deletion kclvm/cmd/src/lint.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ use anyhow::Result;
use clap::ArgMatches;
use kclvm_error::Handler;
use kclvm_runner::ExecProgramArgs;
use kclvm_tools::lint::lint_files;
use kclvm_tools::{fix, lint::lint_files};

use crate::settings::must_build_settings;

Expand All @@ -23,11 +23,19 @@ pub fn lint_command(matches: &ArgMatches) -> Result<()> {
args.get_files()
};
let (mut err_handler, mut warning_handler) = (Handler::default(), Handler::default());

(err_handler.diagnostics, warning_handler.diagnostics) =
lint_files(&files, Some(args.get_load_program_options()));
if bool_from_matches(matches, "emit_warning").unwrap_or_default() {
warning_handler.emit()?;
}

if bool_from_matches(matches, "fix").unwrap_or_default() {
let mut diags = vec![];
diags.extend(err_handler.diagnostics.clone());
diags.extend(warning_handler.diagnostics);
fix::fix(diags)?;
}
err_handler.abort_if_any_errors();
Ok(())
}
5 changes: 4 additions & 1 deletion kclvm/error/src/diagnostic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ impl From<Loc> for Position {

impl Diagnostic {
pub fn new(level: Level, message: &str, range: Range) -> Self {
Diagnostic::new_with_code(level, message, None, range, None)
Diagnostic::new_with_code(level, message, None, range, None, None)
}

/// New a diagnostic with error code.
Expand All @@ -106,6 +106,7 @@ impl Diagnostic {
note: Option<&str>,
range: Range,
code: Option<DiagnosticId>,
suggested_replacement: Option<String>,
) -> Self {
Diagnostic {
level,
Expand All @@ -114,6 +115,7 @@ impl Diagnostic {
style: Style::LineAndColumn,
message: message.to_string(),
note: note.map(|s| s.to_string()),
suggested_replacement,
}],
code,
}
Expand All @@ -133,6 +135,7 @@ pub struct Message {
pub style: Style,
pub message: String,
pub note: Option<String>,
pub suggested_replacement: Option<String>,
}

#[derive(Clone, Debug, PartialEq, Eq, Hash)]
Expand Down
2 changes: 2 additions & 0 deletions kclvm/error/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,7 @@ pub enum WarningKind {
/// style: Style::LineAndColumn,
/// message: "Module 'a' imported but unused.".to_string(),
/// note: None,
/// suggested_replacement: Some("".to_string()),
/// }],
/// );
/// for diag in &handler.diagnostics {
Expand All @@ -183,6 +184,7 @@ impl std::fmt::Display for WarningKind {
/// style: Style::LineAndColumn,
/// message: "Module 'a' imported but unused.".to_string(),
/// note: None,
/// suggested_replacement: Some("".to_string()),
/// }],
/// );
/// for diag in &handler.diagnostics {
Expand Down
19 changes: 17 additions & 2 deletions kclvm/error/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,7 @@ impl Handler {
None,
range,
Some(DiagnosticId::Error(E1001.kind)),
None,
);
self.add_diagnostic(diag);

Expand All @@ -114,6 +115,7 @@ impl Handler {
None,
range,
Some(DiagnosticId::Error(E2G22.kind)),
None,
);
self.add_diagnostic(diag);

Expand All @@ -128,6 +130,7 @@ impl Handler {
None,
range,
Some(DiagnosticId::Error(E2L23.kind)),
None,
);
self.add_diagnostic(diag);

Expand All @@ -151,6 +154,7 @@ impl Handler {
/// style: Style::LineAndColumn,
/// message: "Invalid syntax: expected '+', got '-'".to_string(),
/// note: None,
/// suggested_replacement: Some("".to_string()),
/// }
/// ]);
/// ```
Expand All @@ -175,6 +179,7 @@ impl Handler {
/// style: Style::LineAndColumn,
/// message: "Module 'a' imported but unused.".to_string(),
/// note: None,
/// suggested_replacement: Some("".to_string()),
/// }],
/// );
/// ```
Expand Down Expand Up @@ -211,7 +216,7 @@ impl Handler {
/// ```
/// use kclvm_error::*;
/// let mut handler = Handler::default();
/// handler.add_diagnostic(Diagnostic::new_with_code(Level::Error, "error message", None, (Position::dummy_pos(), Position::dummy_pos()), Some(DiagnosticId::Error(E1001.kind))));
/// handler.add_diagnostic(Diagnostic::new_with_code(Level::Error, "error message", None, (Position::dummy_pos(), Position::dummy_pos()), Some(DiagnosticId::Error(E1001.kind)), None));
/// ```
#[inline]
pub fn add_diagnostic(&mut self, diagnostic: Diagnostic) -> &mut Self {
Expand All @@ -235,7 +240,14 @@ impl From<PanicInfo> for Diagnostic {
line: panic_info.kcl_line as u64,
column: None,
};
Diagnostic::new_with_code(Level::Error, &panic_msg, None, (pos.clone(), pos), None)
Diagnostic::new_with_code(
Level::Error,
&panic_msg,
None,
(pos.clone(), pos),
None,
None,
)
} else {
let mut backtrace_msg = "backtrace:\n".to_string();
let mut backtrace = panic_info.backtrace.clone();
Expand All @@ -261,6 +273,7 @@ impl From<PanicInfo> for Diagnostic {
Some(&backtrace_msg),
(pos.clone(), pos),
None,
None,
)
};

Expand All @@ -278,6 +291,7 @@ impl From<PanicInfo> for Diagnostic {
None,
(pos.clone(), pos),
None,
None,
);
config_meta_diag.messages.append(&mut diag.messages);
config_meta_diag
Expand Down Expand Up @@ -334,6 +348,7 @@ impl ParseError {
None,
(pos.clone(), pos),
Some(DiagnosticId::Error(ErrorKind::InvalidSyntax)),
None,
))
}
}
Expand Down
3 changes: 3 additions & 0 deletions kclvm/parser/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -386,6 +386,7 @@ impl Loader {
pkg_path
),
note: None,
suggested_replacement: None,
}],
);
return Ok(None);
Expand All @@ -402,6 +403,7 @@ impl Loader {
style: Style::Line,
message: format!("pkgpath {} not found in the program", pkg_path),
note: None,
suggested_replacement: None,
}],
);
return Ok(None);
Expand Down Expand Up @@ -498,6 +500,7 @@ impl Loader {
style: Style::Line,
message: format!("the plugin package `{}` is not found, please confirm if plugin mode is enabled", pkgpath),
note: None,
suggested_replacement: None,
}],
);
}
Expand Down
3 changes: 3 additions & 0 deletions kclvm/sema/src/lint/lints_def.rs
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ impl LintPass for ImportPosition {
note: Some(
"Consider moving tihs statement to the top of the file".to_string(),
),
suggested_replacement: None,
}],
);
}
Expand Down Expand Up @@ -109,6 +110,7 @@ impl LintPass for UnusedImport {
style: Style::Line,
message: format!("Module '{}' imported but unused", scope_obj.name),
note: Some("Consider removing this statement".to_string()),
suggested_replacement: Some("".to_string()),
}],
);
}
Expand Down Expand Up @@ -163,6 +165,7 @@ impl LintPass for ReImport {
&import_stmt.name
),
note: Some("Consider removing this statement".to_string()),
suggested_replacement: Some("".to_string()),
}],
);
} else {
Expand Down
1 change: 1 addition & 0 deletions kclvm/sema/src/resolver/attr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ impl<'ctx> Resolver<'ctx> {
attr_ty.ty_str()
),
note: None,
suggested_replacement: None,
}],
);
}
Expand Down
1 change: 1 addition & 0 deletions kclvm/sema/src/resolver/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -545,6 +545,7 @@ impl<'ctx> Resolver<'ctx> {
val_ty.ty_str()
),
note: None,
suggested_replacement: None,
}],
);
}
Expand Down
18 changes: 18 additions & 0 deletions kclvm/sema/src/resolver/global.rs
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ impl<'ctx> Resolver<'ctx> {
style: Style::LineAndColumn,
message: format!("unique key error name '{}'", name),
note: None,
suggested_replacement: None,
}],
);
continue;
Expand Down Expand Up @@ -182,6 +183,7 @@ impl<'ctx> Resolver<'ctx> {
style: Style::Line,
message: format!("pkgpath {} not found in the program", self.ctx.pkgpath),
note: None,
suggested_replacement: None,
}],
);
}
Expand Down Expand Up @@ -237,6 +239,7 @@ impl<'ctx> Resolver<'ctx> {
name
),
note: None,
suggested_replacement: None,
},
Message {
range: self
Expand All @@ -253,6 +256,7 @@ impl<'ctx> Resolver<'ctx> {
"change the variable name to '_{}' to make it mutable",
name
)),
suggested_replacement: None,
},
],
);
Expand All @@ -276,12 +280,14 @@ impl<'ctx> Resolver<'ctx> {
obj.ty.ty_str()
),
note: None,
suggested_replacement: None,
},
Message {
range: obj.get_span_pos(),
style: Style::LineAndColumn,
message: format!("expected {}", obj.ty.ty_str()),
note: None,
suggested_replacement: None,
},
],
);
Expand Down Expand Up @@ -349,6 +355,7 @@ impl<'ctx> Resolver<'ctx> {
ty.ty_str()
),
note: None,
suggested_replacement: None,
}],
);
None
Expand All @@ -372,6 +379,7 @@ impl<'ctx> Resolver<'ctx> {
style: Style::LineAndColumn,
message: "only schema mixin can inherit from protocol".to_string(),
note: None,
suggested_replacement: None,
}],
);
return None;
Expand All @@ -393,6 +401,7 @@ impl<'ctx> Resolver<'ctx> {
ty.ty_str()
),
note: None,
suggested_replacement: None,
}],
);
None
Expand Down Expand Up @@ -426,6 +435,7 @@ impl<'ctx> Resolver<'ctx> {
ty.ty_str()
),
note: None,
suggested_replacement: None,
}],
);
None
Expand Down Expand Up @@ -462,6 +472,7 @@ impl<'ctx> Resolver<'ctx> {
style: Style::LineAndColumn,
message: format!("schema protocol name must end with '{}'", PROTOCOL_SUFFIX),
note: None,
suggested_replacement: None,
}],
);
}
Expand All @@ -482,6 +493,7 @@ impl<'ctx> Resolver<'ctx> {
style: Style::LineAndColumn,
message: format!("mixin inheritance {} is prohibited", parent_name),
note: None,
suggested_replacement: None,
}],
);
}
Expand All @@ -500,6 +512,7 @@ impl<'ctx> Resolver<'ctx> {
style: Style::LineAndColumn,
message: format!("index signature attribute name '{}' cannot have the same name as schema attributes", index_sign_name),
note: None,
suggested_replacement: None,
}],
);
}
Expand All @@ -524,6 +537,7 @@ impl<'ctx> Resolver<'ctx> {
style: Style::LineAndColumn,
message: format!("invalid index signature key type: '{}'", key_ty.ty_str()),
note: None,
suggested_replacement: None,
}],
);
}
Expand Down Expand Up @@ -666,6 +680,7 @@ impl<'ctx> Resolver<'ctx> {
style: Style::LineAndColumn,
message: format!("the type '{}' of schema attribute '{}' does not meet the index signature definition {}", ty.ty_str(), name, index_signature_obj.ty_str()),
note: None,
suggested_replacement: None,
}],
);
}
Expand All @@ -686,6 +701,7 @@ impl<'ctx> Resolver<'ctx> {
mixin_names[mixin_names.len() - 1]
),
note: None,
suggested_replacement: None,
}],
);
}
Expand All @@ -707,6 +723,7 @@ impl<'ctx> Resolver<'ctx> {
ty.ty_str()
),
note: None,
suggested_replacement: None,
}],
);
None
Expand Down Expand Up @@ -832,6 +849,7 @@ impl<'ctx> Resolver<'ctx> {
style: Style::LineAndColumn,
message: format!("illegal rule type '{}'", ty.ty_str()),
note: None,
suggested_replacement: None,
}],
);
None
Expand Down
2 changes: 2 additions & 0 deletions kclvm/sema/src/resolver/import.rs
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ impl<'ctx> Resolver<'ctx> {
real_path.to_str().unwrap()
),
note: None,
suggested_replacement: None,
}],
);
} else {
Expand All @@ -60,6 +61,7 @@ impl<'ctx> Resolver<'ctx> {
file
),
note: None,
suggested_replacement: None,
}],
);
}
Expand Down
Loading

0 comments on commit eeff00e

Please sign in to comment.