From eeff00e4db1e4fc4db8240967eb3f51db93c4fb7 Mon Sep 17 00:00:00 2001 From: He1pa <56333845+He1pa@users.noreply.github.com> Date: Fri, 22 Sep 2023 18:44:04 +0800 Subject: [PATCH] feat: auto fix tools (#719) * 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 --- kclvm/cmd/src/lib.rs | 3 +- kclvm/cmd/src/lint.rs | 10 +- kclvm/error/src/diagnostic.rs | 5 +- kclvm/error/src/error.rs | 2 + kclvm/error/src/lib.rs | 19 +- kclvm/parser/src/lib.rs | 3 + kclvm/sema/src/lint/lints_def.rs | 3 + kclvm/sema/src/resolver/attr.rs | 1 + kclvm/sema/src/resolver/config.rs | 1 + kclvm/sema/src/resolver/global.rs | 18 ++ kclvm/sema/src/resolver/import.rs | 2 + kclvm/sema/src/resolver/node.rs | 2 + kclvm/sema/src/resolver/para.rs | 1 + kclvm/sema/src/resolver/schema.rs | 2 + kclvm/sema/src/resolver/tests.rs | 3 + kclvm/sema/src/resolver/ty.rs | 2 + kclvm/sema/src/resolver/var.rs | 2 + kclvm/tools/src/fix/LICENSE | 201 +++++++++++++ kclvm/tools/src/fix/mod.rs | 166 +++++++++++ kclvm/tools/src/fix/replace.rs | 274 ++++++++++++++++++ kclvm/tools/src/fix/test_data/fix_import.k | 5 + kclvm/tools/src/fix/test_data/unused_import.k | 1 + kclvm/tools/src/fix/tests.rs | 42 +++ kclvm/tools/src/lib.rs | 1 + kclvm/tools/src/lint/mod.rs | 4 +- 25 files changed, 767 insertions(+), 6 deletions(-) create mode 100644 kclvm/tools/src/fix/LICENSE create mode 100644 kclvm/tools/src/fix/mod.rs create mode 100644 kclvm/tools/src/fix/replace.rs create mode 100644 kclvm/tools/src/fix/test_data/fix_import.k create mode 100644 kclvm/tools/src/fix/test_data/unused_import.k create mode 100644 kclvm/tools/src/fix/tests.rs diff --git a/kclvm/cmd/src/lib.rs b/kclvm/cmd/src/lib.rs index af51d8ab1..1bc06e22f 100644 --- a/kclvm/cmd/src/lib.rs +++ b/kclvm/cmd/src/lib.rs @@ -79,7 +79,8 @@ pub fn app() -> Command { .arg(arg!(path_selector: -S --path_selector ... "Specify the path selector").num_args(1..)) .arg(arg!(overrides: -O --overrides ... "Specify the configuration override path and value").num_args(1..)) .arg(arg!(target: --target "Specify the target type")) - .arg(arg!(package_map: -E --external ... "Mapping of package name and path where the package is located").num_args(1..)), + .arg(arg!(package_map: -E --external ... "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") diff --git a/kclvm/cmd/src/lint.rs b/kclvm/cmd/src/lint.rs index 044007c63..26e2d55a4 100644 --- a/kclvm/cmd/src/lint.rs +++ b/kclvm/cmd/src/lint.rs @@ -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; @@ -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(()) } diff --git a/kclvm/error/src/diagnostic.rs b/kclvm/error/src/diagnostic.rs index 6e7f96df5..71afa8fc2 100644 --- a/kclvm/error/src/diagnostic.rs +++ b/kclvm/error/src/diagnostic.rs @@ -96,7 +96,7 @@ impl From 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. @@ -106,6 +106,7 @@ impl Diagnostic { note: Option<&str>, range: Range, code: Option, + suggested_replacement: Option, ) -> Self { Diagnostic { level, @@ -114,6 +115,7 @@ impl Diagnostic { style: Style::LineAndColumn, message: message.to_string(), note: note.map(|s| s.to_string()), + suggested_replacement, }], code, } @@ -133,6 +135,7 @@ pub struct Message { pub style: Style, pub message: String, pub note: Option, + pub suggested_replacement: Option, } #[derive(Clone, Debug, PartialEq, Eq, Hash)] diff --git a/kclvm/error/src/error.rs b/kclvm/error/src/error.rs index f3bf4d7e1..ed713616d 100644 --- a/kclvm/error/src/error.rs +++ b/kclvm/error/src/error.rs @@ -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 { @@ -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 { diff --git a/kclvm/error/src/lib.rs b/kclvm/error/src/lib.rs index 7a736cb20..745b86334 100644 --- a/kclvm/error/src/lib.rs +++ b/kclvm/error/src/lib.rs @@ -100,6 +100,7 @@ impl Handler { None, range, Some(DiagnosticId::Error(E1001.kind)), + None, ); self.add_diagnostic(diag); @@ -114,6 +115,7 @@ impl Handler { None, range, Some(DiagnosticId::Error(E2G22.kind)), + None, ); self.add_diagnostic(diag); @@ -128,6 +130,7 @@ impl Handler { None, range, Some(DiagnosticId::Error(E2L23.kind)), + None, ); self.add_diagnostic(diag); @@ -151,6 +154,7 @@ impl Handler { /// style: Style::LineAndColumn, /// message: "Invalid syntax: expected '+', got '-'".to_string(), /// note: None, + /// suggested_replacement: Some("".to_string()), /// } /// ]); /// ``` @@ -175,6 +179,7 @@ impl Handler { /// style: Style::LineAndColumn, /// message: "Module 'a' imported but unused.".to_string(), /// note: None, + /// suggested_replacement: Some("".to_string()), /// }], /// ); /// ``` @@ -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 { @@ -235,7 +240,14 @@ impl From 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(); @@ -261,6 +273,7 @@ impl From for Diagnostic { Some(&backtrace_msg), (pos.clone(), pos), None, + None, ) }; @@ -278,6 +291,7 @@ impl From for Diagnostic { None, (pos.clone(), pos), None, + None, ); config_meta_diag.messages.append(&mut diag.messages); config_meta_diag @@ -334,6 +348,7 @@ impl ParseError { None, (pos.clone(), pos), Some(DiagnosticId::Error(ErrorKind::InvalidSyntax)), + None, )) } } diff --git a/kclvm/parser/src/lib.rs b/kclvm/parser/src/lib.rs index 0cd42ecb8..eed2eb690 100644 --- a/kclvm/parser/src/lib.rs +++ b/kclvm/parser/src/lib.rs @@ -386,6 +386,7 @@ impl Loader { pkg_path ), note: None, + suggested_replacement: None, }], ); return Ok(None); @@ -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); @@ -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, }], ); } diff --git a/kclvm/sema/src/lint/lints_def.rs b/kclvm/sema/src/lint/lints_def.rs index 2816a5178..cc1386a68 100644 --- a/kclvm/sema/src/lint/lints_def.rs +++ b/kclvm/sema/src/lint/lints_def.rs @@ -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, }], ); } @@ -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()), }], ); } @@ -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 { diff --git a/kclvm/sema/src/resolver/attr.rs b/kclvm/sema/src/resolver/attr.rs index 229110f77..4c7fa6c1d 100644 --- a/kclvm/sema/src/resolver/attr.rs +++ b/kclvm/sema/src/resolver/attr.rs @@ -23,6 +23,7 @@ impl<'ctx> Resolver<'ctx> { attr_ty.ty_str() ), note: None, + suggested_replacement: None, }], ); } diff --git a/kclvm/sema/src/resolver/config.rs b/kclvm/sema/src/resolver/config.rs index c5428fc76..5c524aff8 100644 --- a/kclvm/sema/src/resolver/config.rs +++ b/kclvm/sema/src/resolver/config.rs @@ -545,6 +545,7 @@ impl<'ctx> Resolver<'ctx> { val_ty.ty_str() ), note: None, + suggested_replacement: None, }], ); } diff --git a/kclvm/sema/src/resolver/global.rs b/kclvm/sema/src/resolver/global.rs index f7ad894bd..240a90f14 100644 --- a/kclvm/sema/src/resolver/global.rs +++ b/kclvm/sema/src/resolver/global.rs @@ -58,6 +58,7 @@ impl<'ctx> Resolver<'ctx> { style: Style::LineAndColumn, message: format!("unique key error name '{}'", name), note: None, + suggested_replacement: None, }], ); continue; @@ -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, }], ); } @@ -237,6 +239,7 @@ impl<'ctx> Resolver<'ctx> { name ), note: None, + suggested_replacement: None, }, Message { range: self @@ -253,6 +256,7 @@ impl<'ctx> Resolver<'ctx> { "change the variable name to '_{}' to make it mutable", name )), + suggested_replacement: None, }, ], ); @@ -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, }, ], ); @@ -349,6 +355,7 @@ impl<'ctx> Resolver<'ctx> { ty.ty_str() ), note: None, + suggested_replacement: None, }], ); None @@ -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; @@ -393,6 +401,7 @@ impl<'ctx> Resolver<'ctx> { ty.ty_str() ), note: None, + suggested_replacement: None, }], ); None @@ -426,6 +435,7 @@ impl<'ctx> Resolver<'ctx> { ty.ty_str() ), note: None, + suggested_replacement: None, }], ); None @@ -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, }], ); } @@ -482,6 +493,7 @@ impl<'ctx> Resolver<'ctx> { style: Style::LineAndColumn, message: format!("mixin inheritance {} is prohibited", parent_name), note: None, + suggested_replacement: None, }], ); } @@ -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, }], ); } @@ -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, }], ); } @@ -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, }], ); } @@ -686,6 +701,7 @@ impl<'ctx> Resolver<'ctx> { mixin_names[mixin_names.len() - 1] ), note: None, + suggested_replacement: None, }], ); } @@ -707,6 +723,7 @@ impl<'ctx> Resolver<'ctx> { ty.ty_str() ), note: None, + suggested_replacement: None, }], ); None @@ -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 diff --git a/kclvm/sema/src/resolver/import.rs b/kclvm/sema/src/resolver/import.rs index 28a6051fb..a31e4918c 100644 --- a/kclvm/sema/src/resolver/import.rs +++ b/kclvm/sema/src/resolver/import.rs @@ -45,6 +45,7 @@ impl<'ctx> Resolver<'ctx> { real_path.to_str().unwrap() ), note: None, + suggested_replacement: None, }], ); } else { @@ -60,6 +61,7 @@ impl<'ctx> Resolver<'ctx> { file ), note: None, + suggested_replacement: None, }], ); } diff --git a/kclvm/sema/src/resolver/node.rs b/kclvm/sema/src/resolver/node.rs index 401b46555..45db998b7 100644 --- a/kclvm/sema/src/resolver/node.rs +++ b/kclvm/sema/src/resolver/node.rs @@ -214,6 +214,7 @@ impl<'ctx> MutSelfTypedResultWalker<'ctx> for Resolver<'ctx> { style: Style::LineAndColumn, message: format!("Immutable variable '{}' is modified during compiling", name), note: None, + suggested_replacement: None, }]; if let Some(pos) = self.get_global_name_pos(name) { msgs.push(Message { @@ -224,6 +225,7 @@ impl<'ctx> MutSelfTypedResultWalker<'ctx> for Resolver<'ctx> { "change the variable name to '_{}' to make it mutable", name )), + suggested_replacement: None, }) } self.handler.add_error(ErrorKind::ImmutableError, &msgs); diff --git a/kclvm/sema/src/resolver/para.rs b/kclvm/sema/src/resolver/para.rs index 6a213d8f3..61c79d055 100644 --- a/kclvm/sema/src/resolver/para.rs +++ b/kclvm/sema/src/resolver/para.rs @@ -22,6 +22,7 @@ impl<'ctx> Resolver<'ctx> { message: "non-default argument follows default argument" .to_string(), note: Some("A default argument".to_string()), + suggested_replacement: None, }], ); } diff --git a/kclvm/sema/src/resolver/schema.rs b/kclvm/sema/src/resolver/schema.rs index a14980a25..a3f5ffb49 100644 --- a/kclvm/sema/src/resolver/schema.rs +++ b/kclvm/sema/src/resolver/schema.rs @@ -32,6 +32,7 @@ impl<'ctx> Resolver<'ctx> { style: Style::LineAndColumn, message: format!("expected schema type, got {}", ty.ty_str()), note: None, + suggested_replacement: None, }], ); return ty; @@ -120,6 +121,7 @@ impl<'ctx> Resolver<'ctx> { style: Style::LineAndColumn, message: format!("expected rule type, got {}", ty.ty_str()), note: None, + suggested_replacement: None, }], ); return ty; diff --git a/kclvm/sema/src/resolver/tests.rs b/kclvm/sema/src/resolver/tests.rs index d2dcfa1eb..9f7c6267a 100644 --- a/kclvm/sema/src/resolver/tests.rs +++ b/kclvm/sema/src/resolver/tests.rs @@ -313,6 +313,7 @@ fn test_lint() { style: Style::Line, message: format!("Importstmt should be placed at the top of the module"), note: Some("Consider moving tihs statement to the top of the file".to_string()), + suggested_replacement: None, }], ); handler.add_warning( @@ -333,6 +334,7 @@ fn test_lint() { style: Style::Line, message: format!("Module 'a' is reimported multiple times"), note: Some("Consider removing this statement".to_string()), + suggested_replacement: Some("".to_string()), }], ); handler.add_warning( @@ -353,6 +355,7 @@ fn test_lint() { style: Style::Line, message: format!("Module 'import_test.a' imported but unused"), note: Some("Consider removing this statement".to_string()), + suggested_replacement: Some("".to_string()), }], ); for (d1, d2) in resolver diff --git a/kclvm/sema/src/resolver/ty.rs b/kclvm/sema/src/resolver/ty.rs index c7504d6ab..bc2b1956a 100644 --- a/kclvm/sema/src/resolver/ty.rs +++ b/kclvm/sema/src/resolver/ty.rs @@ -98,6 +98,7 @@ impl<'ctx> Resolver<'ctx> { style: Style::LineAndColumn, message: format!("expected {}, got {}", expected_ty.ty_str(), ty.ty_str(),), note: None, + suggested_replacement: None, }]; if let Some(expected_pos) = expected_pos { @@ -110,6 +111,7 @@ impl<'ctx> Resolver<'ctx> { ty.ty_str(), ), note: None, + suggested_replacement: None, }); } self.handler.add_error(ErrorKind::TypeError, &msgs); diff --git a/kclvm/sema/src/resolver/var.rs b/kclvm/sema/src/resolver/var.rs index 9c73a101c..353e3843f 100644 --- a/kclvm/sema/src/resolver/var.rs +++ b/kclvm/sema/src/resolver/var.rs @@ -132,6 +132,7 @@ impl<'ctx> Resolver<'ctx> { style: Style::LineAndColumn, message: format!("Unique key error name '{}'", name), note: None, + suggested_replacement: None, }]; if let Some(pos) = self.get_global_name_pos(name) { msgs.push(Message { @@ -139,6 +140,7 @@ impl<'ctx> Resolver<'ctx> { style: Style::LineAndColumn, message: format!("The variable '{}' is declared here", name), note: None, + suggested_replacement: None, }); } self.handler.add_error(ErrorKind::UniqueKeyError, &msgs); diff --git a/kclvm/tools/src/fix/LICENSE b/kclvm/tools/src/fix/LICENSE new file mode 100644 index 000000000..989e2c59e --- /dev/null +++ b/kclvm/tools/src/fix/LICENSE @@ -0,0 +1,201 @@ +Apache License + Version 2.0, January 2004 + http://www.apache.org/licenses/ + + TERMS AND CONDITIONS FOR USE, REPRODUCTION, AND DISTRIBUTION + + 1. Definitions. + + "License" shall mean the terms and conditions for use, reproduction, + and distribution as defined by Sections 1 through 9 of this document. + + "Licensor" shall mean the copyright owner or entity authorized by + the copyright owner that is granting the License. + + "Legal Entity" shall mean the union of the acting entity and all + other entities that control, are controlled by, or are under common + control with that entity. For the purposes of this definition, + "control" means (i) the power, direct or indirect, to cause the + direction or management of such entity, whether by contract or + otherwise, or (ii) ownership of fifty percent (50%) or more of the + outstanding shares, or (iii) beneficial ownership of such entity. + + "You" (or "Your") shall mean an individual or Legal Entity + exercising permissions granted by this License. + + "Source" form shall mean the preferred form for making modifications, + including but not limited to software source code, documentation + source, and configuration files. + + "Object" form shall mean any form resulting from mechanical + transformation or translation of a Source form, including but + not limited to compiled object code, generated documentation, + and conversions to other media types. + + "Work" shall mean the work of authorship, whether in Source or + Object form, made available under the License, as indicated by a + copyright notice that is included in or attached to the work + (an example is provided in the Appendix below). + + "Derivative Works" shall mean any work, whether in Source or Object + form, that is based on (or derived from) the Work and for which the + editorial revisions, annotations, elaborations, or other modifications + represent, as a whole, an original work of authorship. For the purposes + of this License, Derivative Works shall not include works that remain + separable from, or merely link (or bind by name) to the interfaces of, + the Work and Derivative Works thereof. + + "Contribution" shall mean any work of authorship, including + the original version of the Work and any modifications or additions + to that Work or Derivative Works thereof, that is intentionally + submitted to Licensor for inclusion in the Work by the copyright owner + or by an individual or Legal Entity authorized to submit on behalf of + the copyright owner. For the purposes of this definition, "submitted" + means any form of electronic, verbal, or written communication sent + to the Licensor or its representatives, including but not limited to + communication on electronic mailing lists, source code control systems, + and issue tracking systems that are managed by, or on behalf of, the + Licensor for the purpose of discussing and improving the Work, but + excluding communication that is conspicuously marked or otherwise + designated in writing by the copyright owner as "Not a Contribution." + + "Contributor" shall mean Licensor and any individual or Legal Entity + on behalf of whom a Contribution has been received by Licensor and + subsequently incorporated within the Work. + + 2. Grant of Copyright License. Subject to the terms and conditions of + this License, each Contributor hereby grants to You a perpetual, + worldwide, non-exclusive, no-charge, royalty-free, irrevocable + copyright license to reproduce, prepare Derivative Works of, + publicly display, publicly perform, sublicense, and distribute the + Work and such Derivative Works in Source or Object form. + + 3. Grant of Patent License. Subject to the terms and conditions of + this License, each Contributor hereby grants to You a perpetual, + worldwide, non-exclusive, no-charge, royalty-free, irrevocable + (except as stated in this section) patent license to make, have made, + use, offer to sell, sell, import, and otherwise transfer the Work, + where such license applies only to those patent claims licensable + by such Contributor that are necessarily infringed by their + Contribution(s) alone or by combination of their Contribution(s) + with the Work to which such Contribution(s) was submitted. If You + institute patent litigation against any entity (including a + cross-claim or counterclaim in a lawsuit) alleging that the Work + or a Contribution incorporated within the Work constitutes direct + or contributory patent infringement, then any patent licenses + granted to You under this License for that Work shall terminate + as of the date such litigation is filed. + + 4. Redistribution. You may reproduce and distribute copies of the + Work or Derivative Works thereof in any medium, with or without + modifications, and in Source or Object form, provided that You + meet the following conditions: + + (a) You must give any other recipients of the Work or + Derivative Works a copy of this License; and + + (b) You must cause any modified files to carry prominent notices + stating that You changed the files; and + + (c) You must retain, in the Source form of any Derivative Works + that You distribute, all copyright, patent, trademark, and + attribution notices from the Source form of the Work, + excluding those notices that do not pertain to any part of + the Derivative Works; and + + (d) If the Work includes a "NOTICE" text file as part of its + distribution, then any Derivative Works that You distribute must + include a readable copy of the attribution notices contained + within such NOTICE file, excluding those notices that do not + pertain to any part of the Derivative Works, in at least one + of the following places: within a NOTICE text file distributed + as part of the Derivative Works; within the Source form or + documentation, if provided along with the Derivative Works; or, + within a display generated by the Derivative Works, if and + wherever such third-party notices normally appear. The contents + of the NOTICE file are for informational purposes only and + do not modify the License. You may add Your own attribution + notices within Derivative Works that You distribute, alongside + or as an addendum to the NOTICE text from the Work, provided + that such additional attribution notices cannot be construed + as modifying the License. + + You may add Your own copyright statement to Your modifications and + may provide additional or different license terms and conditions + for use, reproduction, or distribution of Your modifications, or + for any such Derivative Works as a whole, provided Your use, + reproduction, and distribution of the Work otherwise complies with + the conditions stated in this License. + + 5. Submission of Contributions. Unless You explicitly state otherwise, + any Contribution intentionally submitted for inclusion in the Work + by You to the Licensor shall be under the terms and conditions of + this License, without any additional terms or conditions. + Notwithstanding the above, nothing herein shall supersede or modify + the terms of any separate license agreement you may have executed + with Licensor regarding such Contributions. + + 6. Trademarks. This License does not grant permission to use the trade + names, trademarks, service marks, or product names of the Licensor, + except as required for reasonable and customary use in describing the + origin of the Work and reproducing the content of the NOTICE file. + + 7. Disclaimer of Warranty. Unless required by applicable law or + agreed to in writing, Licensor provides the Work (and each + Contributor provides its Contributions) on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or + implied, including, without limitation, any warranties or conditions + of TITLE, NON-INFRINGEMENT, MERCHANTABILITY, or FITNESS FOR A + PARTICULAR PURPOSE. You are solely responsible for determining the + appropriateness of using or redistributing the Work and assume any + risks associated with Your exercise of permissions under this License. + + 8. Limitation of Liability. In no event and under no legal theory, + whether in tort (including negligence), contract, or otherwise, + unless required by applicable law (such as deliberate and grossly + negligent acts) or agreed to in writing, shall any Contributor be + liable to You for damages, including any direct, indirect, special, + incidental, or consequential damages of any character arising as a + result of this License or out of the use or inability to use the + Work (including but not limited to damages for loss of goodwill, + work stoppage, computer failure or malfunction, or any and all + other commercial damages or losses), even if such Contributor + has been advised of the possibility of such damages. + + 9. Accepting Warranty or Additional Liability. While redistributing + the Work or Derivative Works thereof, You may choose to offer, + and charge a fee for, acceptance of support, warranty, indemnity, + or other liability obligations and/or rights consistent with this + License. However, in accepting such obligations, You may act only + on Your own behalf and on Your sole responsibility, not on behalf + of any other Contributor, and only if You agree to indemnify, + defend, and hold each Contributor harmless for any liability + incurred by, or claims asserted against, such Contributor by reason + of your accepting any such warranty or additional liability. + + END OF TERMS AND CONDITIONS + + APPENDIX: How to apply the Apache License to your work. + + To apply the Apache License to your work, attach the following + boilerplate notice, with the fields enclosed by brackets "[]" + replaced with your own identifying information. (Don't include + the brackets!) The text should be enclosed in the appropriate + comment syntax for the file format. We also recommend that a + file or class name and description of purpose be included on the + same "printed page" as the copyright notice for easier + identification within third-party archives. + + Copyright [yyyy] [name of copyright owner] + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. \ No newline at end of file diff --git a/kclvm/tools/src/fix/mod.rs b/kclvm/tools/src/fix/mod.rs new file mode 100644 index 000000000..243dddf69 --- /dev/null +++ b/kclvm/tools/src/fix/mod.rs @@ -0,0 +1,166 @@ +mod replace; +#[cfg(test)] +mod tests; +use anyhow::{ensure, Error}; +use kclvm_error::{diagnostic::Range as KCLRange, Diagnostic}; +use std::collections::HashMap; +use std::fs; +use std::ops::Range; + +/// A structure for handling code fixes. +pub struct CodeFix { + data: replace::Data, +} + +impl CodeFix { + pub fn new(s: &str) -> CodeFix { + CodeFix { + data: replace::Data::new(s.as_bytes()), + } + } + + pub fn apply(&mut self, suggestion: &Suggestion) -> Result<(), Error> { + let snippet = &suggestion.replacement.snippet; + self.data.replace_range( + snippet.range.start, + snippet.range.end.saturating_sub(1), + suggestion.replacement.replacement.as_bytes(), + )?; + Ok(()) + } + + pub fn finish(&self) -> Result { + Ok(String::from_utf8(self.data.to_vec())?) + } +} + +#[derive(Debug, Clone, Hash, PartialEq, Eq)] +/// An error/warning and possible solutions for fixing it +pub struct Suggestion { + pub message: String, + pub replacement: Replacement, +} + +#[derive(Debug, Clone, Hash, PartialEq, Eq)] +pub struct Replacement { + pub snippet: Snippet, + pub replacement: String, +} + +#[derive(Debug, Clone, Hash, PartialEq, Eq)] +pub struct Snippet { + pub file_name: String, + pub range: Range, +} + +pub fn diag_to_suggestion( + diag: Diagnostic, + files: &mut HashMap, +) -> anyhow::Result, Error> { + let mut suggestions = vec![]; + + for msg in &diag.messages { + if let Some(replace) = &msg.suggested_replacement { + let file_name = msg.range.0.filename.clone(); + let src = match files.get(&file_name) { + Some(src) => src.clone(), + None => { + let src = fs::read_to_string(&file_name).unwrap(); + files.insert(file_name, src.clone()); + src + } + }; + suggestions.push(Suggestion { + message: msg.message.clone(), + replacement: Replacement { + snippet: Snippet { + file_name: msg.range.0.filename.clone(), + range: text_range(src.as_str(), &msg.range)?, + }, + replacement: replace.clone(), + }, + }); + } + } + Ok(suggestions) +} + +fn is_newline_at_index(s: &str, index: usize) -> bool { + let length = s.len(); + + if index >= length { + return false; + } + + let bytes = s.as_bytes(); + + if bytes[index] == b'\n' { + return true; + } + #[cfg(target_os = "windows")] + if bytes[index] == b'\r' && index + 1 < length && bytes[index + 1] == b'\n' { + return true; + } + + false +} + +pub(crate) fn text_range(text: &str, range: &KCLRange) -> anyhow::Result, Error> { + let mut lines_length = vec![]; + let lines_text: Vec<&str> = text.split('\n').collect(); + let mut pre_total_length = 0; + + for line in &lines_text { + lines_length.push(pre_total_length); + pre_total_length += line.len() + "\n".len(); + } + + ensure!( + (range.0.line as usize) <= lines_length.len() + && (range.1.line as usize) <= lines_length.len() + ); + + // The KCL diagnostic line is 1-based and the column is 0-based. + let start = + lines_length.get(range.0.line as usize - 1).unwrap() + range.0.column.unwrap_or(0) as usize; + let mut end = + lines_length.get(range.1.line as usize - 1).unwrap() + range.1.column.unwrap_or(0) as usize; + + if is_newline_at_index(text, end) { + if cfg!(windows) { + end += "\r\n".len() + } else { + end += "\n".len() + } + } + + Ok(Range { start, end }) +} + +pub fn fix(diags: Vec) -> Result<(), Error> { + let mut suggestions = vec![]; + let mut source_code = HashMap::new(); + for diag in diags { + suggestions.extend(diag_to_suggestion(diag, &mut source_code)?) + } + + let mut files = HashMap::new(); + for suggestion in suggestions { + let file = suggestion.replacement.snippet.file_name.clone(); + files.entry(file).or_insert_with(Vec::new).push(suggestion); + } + + for (source_file, suggestions) in &files { + println!("fix file: {:?}", source_file); + let source = fs::read_to_string(source_file)?; + let mut fix = CodeFix::new(&source); + for suggestion in suggestions.iter() { + if let Err(e) = fix.apply(suggestion) { + eprintln!("Failed to apply suggestion to {}: {}", source_file, e); + } + } + let fixes = fix.finish()?; + fs::write(source_file, fixes)?; + } + Ok(()) +} diff --git a/kclvm/tools/src/fix/replace.rs b/kclvm/tools/src/fix/replace.rs new file mode 100644 index 000000000..4bc4d5c31 --- /dev/null +++ b/kclvm/tools/src/fix/replace.rs @@ -0,0 +1,274 @@ +//! The following code and tests are copied and modified from +//! [rust-lang/rustfix](https://github.com/rust-lang/rustfix/blob/master/src/replace.rs) +//! +//! A small module giving you a simple container that allows easy and cheap +//! replacement of parts of its content, with the ability to prevent changing +//! the same parts multiple times. + +use anyhow::{anyhow, ensure, Error}; +use std::rc::Rc; + +#[derive(Debug, Clone, PartialEq, Eq)] +enum State { + Initial, + Replaced(Rc<[u8]>), + Inserted(Rc<[u8]>), +} + +impl State { + fn is_inserted(&self) -> bool { + matches!(*self, State::Inserted(..)) + } +} + +#[derive(Debug, Clone, PartialEq, Eq)] +struct Span { + /// Start of this span in parent data + start: usize, + /// up to end including + end: usize, + data: State, +} + +/// A container that allows easily replacing chunks of its data +#[derive(Debug, Clone, Default)] +pub struct Data { + original: Vec, + parts: Vec, +} + +impl Data { + /// Create a new data container from a slice of bytes + pub fn new(data: &[u8]) -> Self { + Data { + original: data.into(), + parts: vec![Span { + data: State::Initial, + start: 0, + end: data.len().saturating_sub(1), + }], + } + } + + /// Render this data as a vector of bytes + pub fn to_vec(&self) -> Vec { + if self.original.is_empty() { + return Vec::new(); + } + + self.parts.iter().fold(Vec::new(), |mut acc, d| { + match d.data { + State::Initial => acc.extend_from_slice(&self.original[d.start..=d.end]), + State::Replaced(ref d) | State::Inserted(ref d) => acc.extend_from_slice(d), + }; + acc + }) + } + + /// Replace a chunk of data with the given slice, erroring when this part + /// was already changed previously. + pub fn replace_range( + &mut self, + from: usize, + up_to_and_including: usize, + data: &[u8], + ) -> Result<(), Error> { + let exclusive_end = up_to_and_including + 1; + + ensure!( + from <= exclusive_end, + "Invalid range {}...{}, start is larger than end", + from, + up_to_and_including + ); + + ensure!( + up_to_and_including <= self.original.len(), + "Invalid range {}...{} given, original data is only {} byte long", + from, + up_to_and_including, + self.original.len() + ); + + let insert_only = from == exclusive_end; + + // Since we error out when replacing an already replaced chunk of data, + // we can take some shortcuts here. For example, there can be no + // overlapping replacements -- we _always_ split a chunk of 'initial' + // data into three[^empty] parts, and there can't ever be two 'initial' + // parts touching. + // + // [^empty]: Leading and trailing ones might be empty if we replace + // the whole chunk. As an optimization and without loss of generality we + // don't add empty parts. + let new_parts = { + let index_of_part_to_split = self + .parts + .iter() + .position(|p| { + !p.data.is_inserted() && p.start <= from && p.end >= up_to_and_including + }) + .ok_or_else(|| { + anyhow!( + "Could not replace range {}...{} in file \ + -- maybe parts of it were already replaced?", + from, + up_to_and_including + ) + })?; + + let part_to_split = &self.parts[index_of_part_to_split]; + + // If this replacement matches exactly the part that we would + // otherwise split then we ignore this for now. This means that you + // can replace the exact same range with the exact same content + // multiple times and we'll process and allow it. + if part_to_split.start == from && part_to_split.end == up_to_and_including { + if let State::Replaced(ref replacement) = part_to_split.data { + if &**replacement == data { + return Ok(()); + } + } + } + + ensure!( + part_to_split.data == State::Initial, + "Cannot replace slice of data that was already replaced" + ); + + let mut new_parts = Vec::with_capacity(self.parts.len() + 2); + + // Previous parts + if let Some(ps) = self.parts.get(..index_of_part_to_split) { + new_parts.extend_from_slice(ps); + } + + // Keep initial data on left side of part + if from > part_to_split.start { + new_parts.push(Span { + start: part_to_split.start, + end: from.saturating_sub(1), + data: State::Initial, + }); + } + + // New part + new_parts.push(Span { + start: from, + end: up_to_and_including, + data: if insert_only { + State::Inserted(data.into()) + } else { + State::Replaced(data.into()) + }, + }); + + // Keep initial data on right side of part + if up_to_and_including < part_to_split.end { + new_parts.push(Span { + start: up_to_and_including + 1, + end: part_to_split.end, + data: State::Initial, + }); + } + + // Following parts + if let Some(ps) = self.parts.get(index_of_part_to_split + 1..) { + new_parts.extend_from_slice(ps); + } + + new_parts + }; + + self.parts = new_parts; + + Ok(()) + } +} + +#[cfg(test)] +mod tests { + use super::*; + fn str(i: &[u8]) -> &str { + ::std::str::from_utf8(i).unwrap() + } + + #[test] + fn replace_some_stuff() { + let mut d = Data::new(b"foo bar baz"); + d.replace_range(4, 6, b"lol").unwrap(); + assert_eq!("foo lol baz", str(&d.to_vec())); + } + + #[test] + fn replace_a_single_char() { + let mut d = Data::new(b"let y = true;"); + d.replace_range(4, 4, b"mut y").unwrap(); + assert_eq!("let mut y = true;", str(&d.to_vec())); + } + + #[test] + fn replace_multiple_lines() { + let mut d = Data::new(b"lorem\nipsum\ndolor"); + + d.replace_range(6, 10, b"lol").unwrap(); + assert_eq!("lorem\nlol\ndolor", str(&d.to_vec())); + + d.replace_range(12, 16, b"lol").unwrap(); + assert_eq!("lorem\nlol\nlol", str(&d.to_vec())); + } + + #[test] + fn replace_multiple_lines_with_insert_only() { + let mut d = Data::new(b"foo!"); + + d.replace_range(3, 2, b"bar").unwrap(); + assert_eq!("foobar!", str(&d.to_vec())); + + d.replace_range(0, 2, b"baz").unwrap(); + assert_eq!("bazbar!", str(&d.to_vec())); + + d.replace_range(3, 3, b"?").unwrap(); + assert_eq!("bazbar?", str(&d.to_vec())); + } + + #[test] + fn replace_invalid_range() { + let mut d = Data::new(b"foo!"); + + assert!(d.replace_range(2, 0, b"bar").is_err()); + assert!(d.replace_range(0, 2, b"bar").is_ok()); + } + + #[test] + fn empty_to_vec_roundtrip() { + let s = ""; + assert_eq!(s.as_bytes(), Data::new(s.as_bytes()).to_vec().as_slice()); + } + + #[test] + #[should_panic(expected = "Cannot replace slice of data that was already replaced")] + fn replace_overlapping_stuff_errs() { + let mut d = Data::new(b"foo bar baz"); + + d.replace_range(4, 6, b"lol").unwrap(); + assert_eq!("foo lol baz", str(&d.to_vec())); + + d.replace_range(4, 6, b"lol2").unwrap(); + } + + #[test] + #[should_panic(expected = "original data is only 3 byte long")] + fn broken_replacements() { + let mut d = Data::new(b"foo"); + d.replace_range(4, 7, b"lol").unwrap(); + } + + #[test] + fn replace_same_twice() { + let mut d = Data::new(b"foo"); + d.replace_range(0, 0, b"b").unwrap(); + d.replace_range(0, 0, b"b").unwrap(); + assert_eq!("boo", str(&d.to_vec())); + } +} diff --git a/kclvm/tools/src/fix/test_data/fix_import.k b/kclvm/tools/src/fix/test_data/fix_import.k new file mode 100644 index 000000000..4632b86c3 --- /dev/null +++ b/kclvm/tools/src/fix/test_data/fix_import.k @@ -0,0 +1,5 @@ +import regex +import math +import regex + +a = math.pow(1, 1) \ No newline at end of file diff --git a/kclvm/tools/src/fix/test_data/unused_import.k b/kclvm/tools/src/fix/test_data/unused_import.k new file mode 100644 index 000000000..02202730e --- /dev/null +++ b/kclvm/tools/src/fix/test_data/unused_import.k @@ -0,0 +1 @@ +import math \ No newline at end of file diff --git a/kclvm/tools/src/fix/tests.rs b/kclvm/tools/src/fix/tests.rs new file mode 100644 index 000000000..a76afbe8a --- /dev/null +++ b/kclvm/tools/src/fix/tests.rs @@ -0,0 +1,42 @@ +use std::fs; + +use crate::lint::lint_files; + +use super::fix; + +#[test] +fn test_lint() { + let (errors, warnings) = lint_files( + &[ + "./src/fix/test_data/fix_import.k", + "./src/fix/test_data/unused_import.k", + ], + None, + ); + assert_eq!(errors.len(), 0); + let mut diags = vec![]; + diags.extend(warnings); + + match fix(diags) { + Ok(_) => { + let src = fs::read_to_string("./src/fix/test_data/fix_import.k").unwrap(); + #[cfg(target_os = "windows")] + assert_eq!(src, "import math\r\n\r\na = math.pow(1, 1)".to_string()); + #[cfg(not(target_os = "windows"))] + assert_eq!(src, "import math\n\na = math.pow(1, 1)".to_string()); + fs::write( + "./src/fix/test_data/fix_import.k", + r#"import regex +import math +import regex + +a = math.pow(1, 1)"#, + ) + .unwrap(); + let src = fs::read_to_string("./src/fix/test_data/unused_import.k").unwrap(); + assert_eq!(src, "".to_string()); + fs::write("./src/fix/test_data/unused_import.k", r#"import math"#).unwrap(); + } + Err(e) => panic!("fix failed: {:?}", e), + } +} diff --git a/kclvm/tools/src/lib.rs b/kclvm/tools/src/lib.rs index 6ec98222f..fc86da6cc 100644 --- a/kclvm/tools/src/lib.rs +++ b/kclvm/tools/src/lib.rs @@ -1,3 +1,4 @@ +pub mod fix; pub mod format; pub mod lint; pub mod util; diff --git a/kclvm/tools/src/lint/mod.rs b/kclvm/tools/src/lint/mod.rs index 4b27ef939..d60aac176 100644 --- a/kclvm/tools/src/lint/mod.rs +++ b/kclvm/tools/src/lint/mod.rs @@ -71,7 +71,9 @@ pub fn lint_files( ) -> (IndexSet, IndexSet) { // Parse AST program. let sess = Arc::new(ParseSession::default()); - let mut program = match load_program(sess.clone(), files, opts) { + let mut opts = opts.unwrap_or_default(); + opts.load_plugins = true; + let mut program = match load_program(sess.clone(), files, Some(opts)) { Ok(p) => p, Err(err_str) => { return Handler::default()