From c5e037d3af6b286619ffc7e7aa5d8db9fd3472f7 Mon Sep 17 00:00:00 2001 From: Emanuele Stoppa Date: Fri, 14 Jul 2023 14:56:46 +0100 Subject: [PATCH 1/4] testing infra --- Cargo.toml | 1 + crates/rome_analyze/src/lib.rs | 4 +- crates/rome_js_analyze/Cargo.toml | 5 +- crates/rome_js_analyze/tests/spec_tests.rs | 199 ++----------- crates/rome_js_syntax/src/file_source.rs | 21 ++ crates/rome_js_transform/Cargo.toml | 9 +- crates/rome_js_transform/src/lib.rs | 55 ++++ .../src/transformers/ts_enum.rs | 13 +- crates/rome_js_transform/tests/spec_tests.rs | 168 +++++++++++ .../tests/specs/transformEnum/index.ts | 4 + .../specs/transformEnum/index.ts.snap.new | 25 ++ crates/rome_json_analyze/Cargo.toml | 6 +- crates/rome_json_analyze/tests/spec_tests.rs | 226 ++------------- crates/rome_rowan/src/ast/batch.rs | 2 +- crates/rome_test_utils/Cargo.toml | 24 ++ crates/rome_test_utils/src/lib.rs | 264 ++++++++++++++++++ 16 files changed, 622 insertions(+), 404 deletions(-) create mode 100644 crates/rome_js_transform/tests/spec_tests.rs create mode 100644 crates/rome_js_transform/tests/specs/transformEnum/index.ts create mode 100644 crates/rome_js_transform/tests/specs/transformEnum/index.ts.snap.new create mode 100644 crates/rome_test_utils/Cargo.toml create mode 100644 crates/rome_test_utils/src/lib.rs diff --git a/Cargo.toml b/Cargo.toml index 494d9522e29..4bfe4b8d25b 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -64,6 +64,7 @@ rome_suppression = { version = "0.0.1", path = "./crates/rome_suppres rome_text_edit = { version = "0.0.1", path = "./crates/rome_text_edit" } rome_text_size = { version = "0.0.1", path = "./crates/rome_text_size" } tests_macros = { path = "./crates/tests_macros" } +rome_test_utils = { path = "./crates/rome_test_utils" } # Crates needed in the workspace bitflags = "2.3.1" diff --git a/crates/rome_analyze/src/lib.rs b/crates/rome_analyze/src/lib.rs index 9df55ad1135..1c96c3e153d 100644 --- a/crates/rome_analyze/src/lib.rs +++ b/crates/rome_analyze/src/lib.rs @@ -39,7 +39,9 @@ pub use crate::rule::{ RuleMeta, RuleMetadata, SuppressAction, }; pub use crate::services::{FromServices, MissingServicesDiagnostic, ServiceBag}; -pub use crate::signals::{AnalyzerAction, AnalyzerSignal, DiagnosticSignal}; +pub use crate::signals::{ + AnalyzerAction, AnalyzerSignal, AnalyzerTransformation, DiagnosticSignal, +}; pub use crate::syntax::{Ast, SyntaxVisitor}; pub use crate::visitor::{NodeVisitor, Visitor, VisitorContext, VisitorFinishContext}; diff --git a/crates/rome_js_analyze/Cargo.toml b/crates/rome_js_analyze/Cargo.toml index bd5d8997506..ddf063fdcda 100644 --- a/crates/rome_js_analyze/Cargo.toml +++ b/crates/rome_js_analyze/Cargo.toml @@ -33,13 +33,10 @@ smallvec = { workspace = true } [dev-dependencies] countme = { workspace = true, features = ["enable"] } insta = { workspace = true, features = ["glob"] } -json_comments = "0.2.1" rome_js_parser = { workspace = true, features = ["tests"] } -rome_json_parser = { path = "../rome_json_parser" } -rome_service = { path = "../rome_service" } rome_text_edit = { workspace = true } -similar = "2.1.0" tests_macros = { workspace = true } +rome_test_utils = { workspace = true } [features] schemars = ["dep:schemars"] diff --git a/crates/rome_js_analyze/tests/spec_tests.rs b/crates/rome_js_analyze/tests/spec_tests.rs index 72bcedafb00..f4ea2ff00a8 100644 --- a/crates/rome_js_analyze/tests/spec_tests.rs +++ b/crates/rome_js_analyze/tests/spec_tests.rs @@ -1,42 +1,19 @@ -use json_comments::StripComments; -use rome_analyze::{ - AnalysisFilter, AnalyzerAction, AnalyzerOptions, ControlFlow, Never, RuleFilter, -}; -use rome_console::{ - fmt::{Formatter, Termcolor}, - markup, Markup, -}; -use rome_deserialize::json::deserialize_from_json_str; +use rome_analyze::{AnalysisFilter, AnalyzerAction, ControlFlow, Never, RuleFilter}; use rome_diagnostics::advice::CodeSuggestionAdvice; -use rome_diagnostics::termcolor::NoColor; -use rome_diagnostics::{DiagnosticExt, Error, PrintDiagnostic, Severity}; -use rome_js_parser::{ - parse, - test_utils::{assert_errors_are_absent, has_bogus_nodes_or_empty_slots}, - JsParserOptions, -}; +use rome_diagnostics::{DiagnosticExt, Severity}; +use rome_js_parser::{parse, JsParserOptions}; use rome_js_syntax::{JsFileSource, JsLanguage}; -use rome_service::configuration::to_analyzer_configuration; -use rome_service::settings::WorkspaceSettings; -use rome_service::Configuration; -use similar::TextDiff; -use std::{ - ffi::OsStr, fmt::Write, fs::read_to_string, os::raw::c_int, path::Path, slice, sync::Once, +use rome_rowan::AstNode; +use rome_test_utils::{ + assert_errors_are_absent, code_fix_to_string, create_analyzer_options, diagnostic_to_string, + has_bogus_nodes_or_empty_slots, parse_test_path, register_leak_checker, scripts_from_json, + write_analyzer_snapshot, CheckActionType, }; +use std::{ffi::OsStr, fs::read_to_string, path::Path, slice}; tests_macros::gen_tests! {"tests/specs/**/*.{cjs,js,jsx,tsx,ts,json,jsonc}", crate::run_test, "module"} tests_macros::gen_tests! {"tests/suppression/**/*.{cjs,js,jsx,tsx,ts,json,jsonc}", crate::run_suppression_test, "module"} -fn scripts_from_json(extension: &OsStr, input_code: &str) -> Option> { - if extension == "json" || extension == "jsonc" { - let input_code = StripComments::new(input_code.as_bytes()); - let scripts: Vec = serde_json::from_reader(input_code).ok()?; - Some(scripts) - } else { - None - } -} - fn run_test(input: &'static str, _: &str, _: &str, _: &str) { register_leak_checker(); @@ -67,7 +44,7 @@ fn run_test(input: &'static str, _: &str, _: &str, _: &str) { .unwrap_or_else(|err| panic!("failed to read {:?}: {:?}", input_file, err)); let quantity_diagnostics = if let Some(scripts) = scripts_from_json(extension, &input_code) { for script in scripts { - write_analysis_to_snapshot( + analyze_and_snap( &mut snapshot, &script, JsFileSource::js_script(), @@ -84,7 +61,7 @@ fn run_test(input: &'static str, _: &str, _: &str, _: &str) { let Ok(source_type) = input_file.try_into() else { return; }; - write_analysis_to_snapshot( + analyze_and_snap( &mut snapshot, &input_code, source_type, @@ -108,19 +85,8 @@ fn run_test(input: &'static str, _: &str, _: &str, _: &str) { } } -enum CheckActionType { - Suppression, - Lint, -} - -impl CheckActionType { - const fn is_suppression(&self) -> bool { - matches!(self, Self::Suppression) - } -} - #[allow(clippy::too_many_arguments)] -pub(crate) fn write_analysis_to_snapshot( +pub(crate) fn analyze_and_snap( snapshot: &mut String, input_code: &str, source_type: JsFileSource, @@ -135,44 +101,7 @@ pub(crate) fn write_analysis_to_snapshot( let mut diagnostics = Vec::new(); let mut code_fixes = Vec::new(); - let mut options = AnalyzerOptions::default(); - // We allow a test file to configure its rule using a special - // file with the same name as the test but with extension ".options.json" - // that configures that specific rule. - let options_file = input_file.with_extension("options.json"); - if let Ok(json) = std::fs::read_to_string(options_file.clone()) { - let deserialized = deserialize_from_json_str::(json.as_str()); - if deserialized.has_errors() { - diagnostics.extend( - deserialized - .into_diagnostics() - .into_iter() - .map(|diagnostic| { - diagnostic_to_string( - options_file.file_stem().unwrap().to_str().unwrap(), - &json, - diagnostic, - ) - }) - .collect::>(), - ); - None - } else { - let configuration = deserialized.into_deserialized(); - let mut settings = WorkspaceSettings::default(); - settings.merge_with_configuration(configuration).unwrap(); - let configuration = - to_analyzer_configuration(&settings.linter, &settings.languages, |_| vec![]); - options = AnalyzerOptions { - configuration, - ..AnalyzerOptions::default() - }; - - Some(json) - } - } else { - None - }; + let options = create_analyzer_options(input_file, &mut diagnostics); let (_, errors) = rome_js_analyze::analyze(&root, filter, &options, source_type, |event| { if let Some(mut diag) = event.diagnostic() { @@ -236,68 +165,16 @@ pub(crate) fn write_analysis_to_snapshot( diagnostics.push(diagnostic_to_string(file_name, input_code, error)); } - writeln!(snapshot, "# Input").unwrap(); - writeln!(snapshot, "```js").unwrap(); - writeln!(snapshot, "{}", input_code).unwrap(); - writeln!(snapshot, "```").unwrap(); - writeln!(snapshot).unwrap(); - - if !diagnostics.is_empty() { - writeln!(snapshot, "# Diagnostics").unwrap(); - for diagnostic in &diagnostics { - writeln!(snapshot, "```").unwrap(); - writeln!(snapshot, "{}", diagnostic).unwrap(); - writeln!(snapshot, "```").unwrap(); - writeln!(snapshot).unwrap(); - } - } - - if !code_fixes.is_empty() { - writeln!(snapshot, "# Actions").unwrap(); - for action in code_fixes { - writeln!(snapshot, "```diff").unwrap(); - writeln!(snapshot, "{}", action).unwrap(); - writeln!(snapshot, "```").unwrap(); - writeln!(snapshot).unwrap(); - } - } + write_analyzer_snapshot( + snapshot, + input_code, + diagnostics.as_slice(), + code_fixes.as_slice(), + ); diagnostics.len() } -/// The test runner for the analyzer is currently designed to have a -/// one-to-one mapping between test case and analyzer rules. -/// So each testing file will be run through the analyzer with only the rule -/// corresponding to the directory name. E.g., `style/useWhile/test.js` -/// will be analyzed with just the `style/useWhile` rule. -fn parse_test_path(file: &Path) -> (&str, &str) { - let rule_folder = file.parent().unwrap(); - let rule_name = rule_folder.file_name().unwrap(); - - let group_folder = rule_folder.parent().unwrap(); - let group_name = group_folder.file_name().unwrap(); - - (group_name.to_str().unwrap(), rule_name.to_str().unwrap()) -} - -fn markup_to_string(markup: Markup) -> String { - let mut buffer = Vec::new(); - let mut write = Termcolor(NoColor::new(&mut buffer)); - let mut fmt = Formatter::new(&mut write); - fmt.write_markup(markup).unwrap(); - - String::from_utf8(buffer).unwrap() -} -#[allow(clippy::let_and_return)] -fn diagnostic_to_string(name: &str, source: &str, diag: Error) -> String { - let error = diag.with_file_path(name).with_file_source_code(source); - let text = markup_to_string(markup! { - {PrintDiagnostic::verbose(&error)} - }); - - text -} - fn check_code_action( path: &Path, source: &str, @@ -329,41 +206,7 @@ fn check_code_action( // Re-parse the modified code and panic if the resulting tree has syntax errors let re_parse = parse(&output, source_type, options); - assert_errors_are_absent(&re_parse, path); -} - -fn code_fix_to_string(source: &str, action: AnalyzerAction) -> String { - let (_, text_edit) = action.mutation.as_text_edits().unwrap_or_default(); - - let output = text_edit.new_string(source); - - let diff = TextDiff::from_lines(source, &output); - - let mut diff = diff.unified_diff(); - diff.context_radius(3); - - diff.to_string() -} - -// Check that all red / green nodes have correctly been released on exit -extern "C" fn check_leaks() { - if let Some(report) = rome_rowan::check_live() { - panic!("\n{report}") - } -} -pub(crate) fn register_leak_checker() { - // Import the atexit function from libc - extern "C" { - fn atexit(f: extern "C" fn()) -> c_int; - } - - // Use an atomic Once to register the check_leaks function to be called - // when the process exits - static ONCE: Once = Once::new(); - ONCE.call_once(|| unsafe { - countme::enable(true); - atexit(check_leaks); - }); + assert_errors_are_absent(&re_parse.tree().syntax(), re_parse.diagnostics(), path); } pub(crate) fn run_suppression_test(input: &'static str, _: &str, _: &str, _: &str) { @@ -383,7 +226,7 @@ pub(crate) fn run_suppression_test(input: &'static str, _: &str, _: &str, _: &st }; let mut snapshot = String::new(); - write_analysis_to_snapshot( + analyze_and_snap( &mut snapshot, &input_code, JsFileSource::jsx(), diff --git a/crates/rome_js_syntax/src/file_source.rs b/crates/rome_js_syntax/src/file_source.rs index 0922d39b7f3..058d0a0cdbd 100644 --- a/crates/rome_js_syntax/src/file_source.rs +++ b/crates/rome_js_syntax/src/file_source.rs @@ -181,6 +181,27 @@ impl JsFileSource { pub const fn is_module(&self) -> bool { self.module_kind.is_module() } + + pub fn as_extension_name(&self) -> &str { + match self.language { + Language::JavaScript => { + if matches!(self.variant, LanguageVariant::Jsx) { + return "jsx"; + } + match self.module_kind { + ModuleKind::Script => "cjs", + ModuleKind::Module => "js", + } + } + Language::TypeScript { .. } => { + if matches!(self.variant, LanguageVariant::Jsx) { + "tsx" + } else { + "ts" + } + } + } + } } impl TryFrom<&Path> for JsFileSource { diff --git a/crates/rome_js_transform/Cargo.toml b/crates/rome_js_transform/Cargo.toml index 73e57591bea..81ff37f6cba 100644 --- a/crates/rome_js_transform/Cargo.toml +++ b/crates/rome_js_transform/Cargo.toml @@ -15,8 +15,13 @@ rome_diagnostics = { workspace = true } rome_js_factory = { workspace = true } rome_js_syntax = { workspace = true } rome_rowan = { workspace = true } +rome_console = {workspace= true} [dev-dependencies] -rome_js_formatter = { path = "../rome_js_formatter" } -rome_js_parser = { path = "../rome_js_parser" } +rome_js_formatter = { workspace = true } +rome_js_parser = { workspace = true } +rome_analyze = { workspace = true } +rome_test_utils = { workspace = true } +tests_macros= { workspace = true } +insta = { workspace = true } diff --git a/crates/rome_js_transform/src/lib.rs b/crates/rome_js_transform/src/lib.rs index ab74e950046..b1905c4745d 100644 --- a/crates/rome_js_transform/src/lib.rs +++ b/crates/rome_js_transform/src/lib.rs @@ -96,3 +96,58 @@ where } pub(crate) type JsBatchMutation = BatchMutation; + +#[cfg(test)] +mod tests { + use rome_analyze::{AnalyzerOptions, Never, RuleCategories, RuleFilter, RuleKey}; + use rome_console::fmt::{Formatter, Termcolor}; + use rome_console::Markup; + use rome_diagnostics::termcolor::NoColor; + use rome_js_parser::{parse, JsParserOptions}; + use rome_js_syntax::{JsFileSource, TextRange, TextSize}; + use std::slice; + + use crate::{transform, AnalysisFilter, ControlFlow}; + + #[ignore] + #[test] + fn quick_test() { + fn markup_to_string(markup: Markup) -> String { + let mut buffer = Vec::new(); + let mut write = Termcolor(NoColor::new(&mut buffer)); + let mut fmt = Formatter::new(&mut write); + fmt.write_markup(markup).unwrap(); + + String::from_utf8(buffer).unwrap() + } + + const SOURCE: &str = r#"enum Foo { Lorem, Ipsum }"#; + + let parsed = parse(SOURCE, JsFileSource::tsx(), JsParserOptions::default()); + + let options = AnalyzerOptions::default(); + let rule_filter = RuleFilter::Rule("transformations", "transformEnum"); + + transform( + &parsed.tree(), + AnalysisFilter { + categories: RuleCategories::TRANSFORMATION, + enabled_rules: Some(slice::from_ref(&rule_filter)), + ..AnalysisFilter::default() + }, + &options, + JsFileSource::tsx(), + |signal| { + for transformation in signal.transformations() { + dbg!(transformation.mutation.as_text_edits().unwrap_or_default()); + let new_code = transformation.mutation.commit(); + eprintln!("{new_code}"); + } + + ControlFlow::::Continue(()) + }, + ); + + // assert_eq!(error_ranges.as_slice(), &[]); + } +} diff --git a/crates/rome_js_transform/src/transformers/ts_enum.rs b/crates/rome_js_transform/src/transformers/ts_enum.rs index e29ed808b86..4de4404722f 100644 --- a/crates/rome_js_transform/src/transformers/ts_enum.rs +++ b/crates/rome_js_transform/src/transformers/ts_enum.rs @@ -15,8 +15,8 @@ use rome_js_syntax::{ AnyJsAssignment, AnyJsAssignmentPattern, AnyJsBinding, AnyJsBindingPattern, AnyJsCallArgument, AnyJsExpression, AnyJsFormalParameter, AnyJsLiteralExpression, AnyJsModuleItem, AnyJsParameter, AnyJsStatement, JsAssignmentExpression, JsComputedMemberAssignment, JsExpressionStatement, - JsFunctionExpression, JsLogicalExpression, JsModule, JsStatementList, JsVariableStatement, - TsEnumDeclaration, T, + JsFunctionExpression, JsLogicalExpression, JsModule, JsModuleItemList, JsStatementList, + JsVariableStatement, TsEnumDeclaration, T, }; use rome_rowan::{AstNode, BatchMutationExt, TriviaPieceKind}; @@ -56,10 +56,10 @@ impl Rule for TsEnum { fn transform(ctx: &RuleContext, state: &Self::State) -> Option { let node = ctx.query(); let mut mutation = node.clone().begin(); - let parent = node.syntax().grand_parent(); + let parent = node.syntax().parent(); if let Some(parent) = parent { - if let Some(module) = JsModule::cast(parent) { + if let Some(module_list) = JsModuleItemList::cast(parent) { let variable = make_variable(state); let function = make_function_caller(state); let statements = vec![ @@ -68,9 +68,8 @@ impl Rule for TsEnum { function, )), ]; - let modules = js_module_item_list(statements.into_iter()); - let new_modules = module.clone().with_items(modules); - mutation.replace_element(module.into(), new_modules.into()); + let new_modules_list = js_module_item_list(statements.into_iter()); + mutation.replace_node(module_list, new_modules_list); } } diff --git a/crates/rome_js_transform/tests/spec_tests.rs b/crates/rome_js_transform/tests/spec_tests.rs new file mode 100644 index 00000000000..fc789942b5f --- /dev/null +++ b/crates/rome_js_transform/tests/spec_tests.rs @@ -0,0 +1,168 @@ +use rome_analyze::{AnalysisFilter, AnalyzerTransformation, ControlFlow, Never, RuleFilter}; +use rome_js_formatter::context::JsFormatOptions; +use rome_js_formatter::format_node; +use rome_js_parser::{parse, JsParserOptions}; +use rome_js_syntax::{JsFileSource, JsLanguage}; +use rome_rowan::AstNode; +use rome_test_utils::{ + assert_errors_are_absent, create_analyzer_options, diagnostic_to_string, + has_bogus_nodes_or_empty_slots, register_leak_checker, scripts_from_json, + write_transformation_snapshot, +}; +use std::{ffi::OsStr, fs::read_to_string, path::Path, slice}; + +tests_macros::gen_tests! {"tests/specs/**/*.{cjs,js,jsx,tsx,ts,json,jsonc}", crate::run_test, "module"} + +fn run_test(input: &'static str, _: &str, _: &str, _: &str) { + register_leak_checker(); + + let input_file = Path::new(input); + let file_name = input_file.file_name().and_then(OsStr::to_str).unwrap(); + + let rule_folder = input_file.parent().unwrap(); + let rule = rule_folder.file_name().unwrap().to_str().unwrap(); + + if rule == "specs" { + panic!("the test file must be placed in the {rule}/// directory"); + } + if rome_js_transform::metadata() + .find_rule("transformations", rule) + .is_none() + { + panic!("could not find rule transformations/{rule}"); + } + + let rule_filter = RuleFilter::Rule("transformations", rule); + let filter = AnalysisFilter { + enabled_rules: Some(slice::from_ref(&rule_filter)), + ..AnalysisFilter::default() + }; + + let mut snapshot = String::new(); + let extension = input_file.extension().unwrap_or_default(); + + let input_code = read_to_string(input_file) + .unwrap_or_else(|err| panic!("failed to read {:?}: {:?}", input_file, err)); + let quantity_diagnostics = if let Some(scripts) = scripts_from_json(extension, &input_code) { + for script in scripts { + analyze_and_snap( + &mut snapshot, + &script, + JsFileSource::js_script(), + filter, + file_name, + input_file, + JsParserOptions::default(), + ); + } + + 0 + } else { + let Ok(source_type) = input_file.try_into() else { + return; + }; + analyze_and_snap( + &mut snapshot, + &input_code, + source_type, + filter, + file_name, + input_file, + JsParserOptions::default(), + ) + }; + + insta::with_settings!({ + prepend_module_to_snapshot => false, + snapshot_path => input_file.parent().unwrap(), + }, { + insta::assert_snapshot!(file_name, snapshot, file_name); + }); + + if input_code.contains("/* should not generate diagnostics */") && quantity_diagnostics > 0 { + panic!("This test should not generate diagnostics"); + } +} + +#[allow(clippy::too_many_arguments)] +pub(crate) fn analyze_and_snap( + snapshot: &mut String, + input_code: &str, + source_type: JsFileSource, + filter: AnalysisFilter, + file_name: &str, + input_file: &Path, + parser_options: JsParserOptions, +) -> usize { + let parsed = parse(input_code, source_type, parser_options.clone()); + let root = parsed.tree(); + + let mut diagnostics = Vec::new(); + let options = create_analyzer_options(input_file, &mut diagnostics); + + let mut transformations = vec![]; + let (_, errors) = rome_js_transform::transform(&root, filter, &options, source_type, |event| { + for transformation in event.transformations() { + check_transformation( + input_file, + input_code, + source_type, + &transformation, + parser_options.clone(), + ); + let node = transformation.mutation.commit(); + + let formatted = format_node(JsFormatOptions::new(source_type), &node).unwrap(); + + transformations.push(formatted.print().unwrap().as_code().to_string()); + } + ControlFlow::::Continue(()) + }); + + for error in errors { + diagnostics.push(diagnostic_to_string(file_name, input_code, error)); + } + + write_transformation_snapshot( + snapshot, + input_code, + transformations.as_slice(), + source_type.as_extension_name(), + ); + + diagnostics.len() +} + +fn check_transformation( + path: &Path, + source: &str, + source_type: JsFileSource, + transformation: &AnalyzerTransformation, + options: JsParserOptions, +) { + let (_, text_edit) = transformation.mutation.as_text_edits().unwrap_or_default(); + + let output = text_edit.new_string(source); + + let new_tree = transformation.mutation.clone().commit(); + + // Checks that applying the text edits returned by the BatchMutation + // returns the same code as printing the modified syntax tree + assert_eq!(new_tree.to_string(), output); + + if has_bogus_nodes_or_empty_slots(&new_tree) { + panic!( + "modified tree has bogus nodes or empty slots:\n{new_tree:#?} \n\n {}", + new_tree + ) + } + + // Checks the returned tree contains no missing children node + if format!("{new_tree:?}").contains("missing (required)") { + panic!("modified tree has missing children:\n{new_tree:#?}") + } + + // Re-parse the modified code and panic if the resulting tree has syntax errors + let re_parse = parse(&output, source_type, options); + assert_errors_are_absent(&re_parse.tree().syntax(), re_parse.diagnostics(), path); +} diff --git a/crates/rome_js_transform/tests/specs/transformEnum/index.ts b/crates/rome_js_transform/tests/specs/transformEnum/index.ts new file mode 100644 index 00000000000..ea67b1d0e4d --- /dev/null +++ b/crates/rome_js_transform/tests/specs/transformEnum/index.ts @@ -0,0 +1,4 @@ +enum Status { + Enabled, + Disabled +} diff --git a/crates/rome_js_transform/tests/specs/transformEnum/index.ts.snap.new b/crates/rome_js_transform/tests/specs/transformEnum/index.ts.snap.new new file mode 100644 index 00000000000..0cd1a754541 --- /dev/null +++ b/crates/rome_js_transform/tests/specs/transformEnum/index.ts.snap.new @@ -0,0 +1,25 @@ +--- +source: crates/rome_js_transform/tests/spec_tests.rs +assertion_line: 79 +expression: index.ts +--- +# Input +```ts +enum Status { + Enabled, + Disabled +} + +``` + +# Transformations +```ts +var Status; +(function (Status) { + Status[(Status["Enabled"] = 0)] = "Enabled"; + Status[(Status["Disabled"] = 0)] = "Disabled"; +})(Status || (Status = {})); + +``` + + diff --git a/crates/rome_json_analyze/Cargo.toml b/crates/rome_json_analyze/Cargo.toml index 639a90ff8f2..5f551556d5e 100644 --- a/crates/rome_json_analyze/Cargo.toml +++ b/crates/rome_json_analyze/Cargo.toml @@ -17,13 +17,9 @@ rome_json_syntax = { workspace = true } rome_rowan = { workspace = true } [dev-dependencies] -countme = { workspace = true, features = ["enable"] } insta = { workspace = true, features = ["glob"] } -json_comments = "0.2.1" -rome_deserialize = { workspace = true } rome_json_factory = { workspace = true } rome_json_parser = { workspace = true } rome_service = { workspace = true } -rome_text_edit = { workspace = true } -similar = "2.1.0" tests_macros = { workspace = true } +rome_test_utils = { workspace = true } diff --git a/crates/rome_json_analyze/tests/spec_tests.rs b/crates/rome_json_analyze/tests/spec_tests.rs index af746f24f30..85808895577 100644 --- a/crates/rome_json_analyze/tests/spec_tests.rs +++ b/crates/rome_json_analyze/tests/spec_tests.rs @@ -1,25 +1,15 @@ -use rome_analyze::{ - AnalysisFilter, AnalyzerAction, AnalyzerOptions, ControlFlow, Never, RuleFilter, -}; -use rome_console::{ - fmt::{Formatter, Termcolor}, - markup, Markup, -}; -use rome_deserialize::json::deserialize_from_json_str; +use rome_analyze::{AnalysisFilter, AnalyzerAction, ControlFlow, Never, RuleFilter}; use rome_diagnostics::advice::CodeSuggestionAdvice; -use rome_diagnostics::termcolor::{Buffer, NoColor}; -use rome_diagnostics::{DiagnosticExt, Error, PrintDiagnostic, Severity}; -use rome_json_parser::{parse_json, JsonParse, JsonParserOptions}; -use rome_json_syntax::{JsonLanguage, JsonSyntaxNode}; -use rome_rowan::SyntaxKind; -use rome_rowan::SyntaxSlot; -use rome_service::configuration::to_analyzer_configuration; -use rome_service::settings::WorkspaceSettings; -use rome_service::Configuration; -use similar::TextDiff; -use std::{ - ffi::OsStr, fmt::Write, fs::read_to_string, os::raw::c_int, path::Path, slice, sync::Once, +use rome_diagnostics::{DiagnosticExt, Severity}; +use rome_json_parser::parse_json; +use rome_json_syntax::JsonLanguage; +use rome_rowan::AstNode; +use rome_test_utils::{ + assert_errors_are_absent, code_fix_to_string, create_analyzer_options, diagnostic_to_string, + has_bogus_nodes_or_empty_slots, parse_test_path, register_leak_checker, + write_analyzer_snapshot, }; +use std::{ffi::OsStr, fs::read_to_string, path::Path, slice}; tests_macros::gen_tests! {"tests/specs/**/*.{json}", crate::run_test, "module"} @@ -56,7 +46,7 @@ fn run_test(input: &'static str, _: &str, _: &str, _: &str) { .unwrap_or_else(|err| panic!("failed to read {:?}: {:?}", input_file, err)); let quantity_diagnostics = - write_analysis_to_snapshot(&mut snapshot, &input_code, filter, file_name, input_file); + analyze_and_snap(&mut snapshot, &input_code, filter, file_name, input_file); insta::with_settings!({ prepend_module_to_snapshot => false, @@ -70,7 +60,7 @@ fn run_test(input: &'static str, _: &str, _: &str, _: &str) { } } -pub(crate) fn write_analysis_to_snapshot( +pub(crate) fn analyze_and_snap( snapshot: &mut String, input_code: &str, filter: AnalysisFilter, @@ -82,44 +72,7 @@ pub(crate) fn write_analysis_to_snapshot( let mut diagnostics = Vec::new(); let mut code_fixes = Vec::new(); - let mut options = AnalyzerOptions::default(); - // We allow a test file to configure its rule using a special - // file with the same name as the test but with extension ".options.json" - // that configures that specific rule. - let options_file = input_file.with_extension("options.json"); - if let Ok(json) = std::fs::read_to_string(options_file.clone()) { - let deserialized = deserialize_from_json_str::(json.as_str()); - if deserialized.has_errors() { - diagnostics.extend( - deserialized - .into_diagnostics() - .into_iter() - .map(|diagnostic| { - diagnostic_to_string( - options_file.file_stem().unwrap().to_str().unwrap(), - &json, - diagnostic, - ) - }) - .collect::>(), - ); - None - } else { - let configuration = deserialized.into_deserialized(); - let mut settings = WorkspaceSettings::default(); - settings.merge_with_configuration(configuration).unwrap(); - let configuration = - to_analyzer_configuration(&settings.linter, &settings.languages, |_| vec![]); - options = AnalyzerOptions { - configuration, - ..AnalyzerOptions::default() - }; - - Some(json) - } - } else { - None - }; + let options = create_analyzer_options(input_file, &mut diagnostics); let (_, errors) = rome_json_analyze::analyze(&root.value().unwrap(), filter, &options, |event| { @@ -149,69 +102,16 @@ pub(crate) fn write_analysis_to_snapshot( for error in errors { diagnostics.push(diagnostic_to_string(file_name, input_code, error)); } - - writeln!(snapshot, "# Input").unwrap(); - writeln!(snapshot, "```js").unwrap(); - writeln!(snapshot, "{}", input_code).unwrap(); - writeln!(snapshot, "```").unwrap(); - writeln!(snapshot).unwrap(); - - if !diagnostics.is_empty() { - writeln!(snapshot, "# Diagnostics").unwrap(); - for diagnostic in &diagnostics { - writeln!(snapshot, "```").unwrap(); - writeln!(snapshot, "{}", diagnostic).unwrap(); - writeln!(snapshot, "```").unwrap(); - writeln!(snapshot).unwrap(); - } - } - - if !code_fixes.is_empty() { - writeln!(snapshot, "# Actions").unwrap(); - for action in code_fixes { - writeln!(snapshot, "```diff").unwrap(); - writeln!(snapshot, "{}", action).unwrap(); - writeln!(snapshot, "```").unwrap(); - writeln!(snapshot).unwrap(); - } - } + write_analyzer_snapshot( + snapshot, + input_code, + diagnostics.as_slice(), + code_fixes.as_slice(), + ); diagnostics.len() } -/// The test runner for the analyzer is currently designed to have a -/// one-to-one mapping between test case and analyzer rules. -/// So each testing file will be run through the analyzer with only the rule -/// corresponding to the directory name. E.g., `style/useWhile/test.js` -/// will be analyzed with just the `style/useWhile` rule. -fn parse_test_path(file: &Path) -> (&str, &str) { - let rule_folder = file.parent().unwrap(); - let rule_name = rule_folder.file_name().unwrap(); - - let group_folder = rule_folder.parent().unwrap(); - let group_name = group_folder.file_name().unwrap(); - - (group_name.to_str().unwrap(), rule_name.to_str().unwrap()) -} - -fn markup_to_string(markup: Markup) -> String { - let mut buffer = Vec::new(); - let mut write = Termcolor(NoColor::new(&mut buffer)); - let mut fmt = Formatter::new(&mut write); - fmt.write_markup(markup).unwrap(); - - String::from_utf8(buffer).unwrap() -} -#[allow(clippy::let_and_return)] -fn diagnostic_to_string(name: &str, source: &str, diag: Error) -> String { - let error = diag.with_file_path(name).with_file_source_code(source); - let text = markup_to_string(markup! { - {PrintDiagnostic::verbose(&error)} - }); - - text -} - fn check_code_action(path: &Path, source: &str, action: &AnalyzerAction) { let (_, text_edit) = action.mutation.as_text_edits().unwrap_or_default(); @@ -237,91 +137,5 @@ fn check_code_action(path: &Path, source: &str, action: &AnalyzerAction) -> String { - let (_, text_edit) = action.mutation.as_text_edits().unwrap_or_default(); - - let output = text_edit.new_string(source); - - let diff = TextDiff::from_lines(source, &output); - - let mut diff = diff.unified_diff(); - diff.context_radius(3); - - diff.to_string() -} - -// Check that all red / green nodes have correctly been released on exit -extern "C" fn check_leaks() { - if let Some(report) = rome_rowan::check_live() { - panic!("\n{report}") - } -} -pub(crate) fn register_leak_checker() { - // Import the atexit function from libc - extern "C" { - fn atexit(f: extern "C" fn()) -> c_int; - } - - // Use an atomic Once to register the check_leaks function to be called - // when the process exits - static ONCE: Once = Once::new(); - ONCE.call_once(|| unsafe { - countme::enable(true); - atexit(check_leaks); - }); -} - -/// This check is used in the parser test to ensure it doesn't emit -/// bogus nodes without diagnostics, and in the analyzer tests to -/// check the syntax trees resulting from code actions are correct -pub fn has_bogus_nodes_or_empty_slots(node: &JsonSyntaxNode) -> bool { - node.descendants().any(|descendant| { - let kind = descendant.kind(); - if kind.is_bogus() { - return true; - } - - if kind.is_list() { - return descendant - .slots() - .any(|slot| matches!(slot, SyntaxSlot::Empty)); - } - - false - }) -} - -/// This function analyzes the parsing result of a file and panic with a -/// detailed message if it contains any error-level diagnostic, bogus nodes, -/// empty list slots or missing required children -pub fn assert_errors_are_absent(program: &JsonParse, path: &Path) { - let syntax = program.syntax(); - let debug_tree = format!("{:?}", program.tree()); - let has_missing_children = debug_tree.contains("missing (required)"); - - if !program.has_errors() && !has_bogus_nodes_or_empty_slots(&syntax) && !has_missing_children { - return; - } - - let mut buffer = Buffer::no_color(); - for diagnostic in program.diagnostics() { - let error = diagnostic - .clone() - .with_file_path(path.to_str().unwrap()) - .with_file_source_code(syntax.to_string()); - Formatter::new(&mut Termcolor(&mut buffer)) - .write_markup(markup! { - {PrintDiagnostic::verbose(&error)} - }) - .unwrap(); - } - - panic!("There should be no errors in the file {:?} but the following errors where present:\n{}\n\nParsed tree:\n{:#?}", - path.display(), - std::str::from_utf8(buffer.as_slice()).unwrap(), - &syntax - ); + assert_errors_are_absent(re_parse.tree().syntax(), re_parse.diagnostics(), path); } diff --git a/crates/rome_rowan/src/ast/batch.rs b/crates/rome_rowan/src/ast/batch.rs index 09da8b6fbea..bed67aebd85 100644 --- a/crates/rome_rowan/src/ast/batch.rs +++ b/crates/rome_rowan/src/ast/batch.rs @@ -310,7 +310,7 @@ where for change in &self.changes { let parent = change.parent.as_ref().unwrap_or(&self.root); - let delete = match parent.slots().nth(change.new_node_slot) { + let delete = match dbg!(parent.slots().nth(change.new_node_slot)) { Some(SyntaxSlot::Node(node)) => node.text_range(), Some(SyntaxSlot::Token(token)) => token.text_range(), _ => continue, diff --git a/crates/rome_test_utils/Cargo.toml b/crates/rome_test_utils/Cargo.toml new file mode 100644 index 00000000000..8a220503fa8 --- /dev/null +++ b/crates/rome_test_utils/Cargo.toml @@ -0,0 +1,24 @@ +[package] +name = "rome_test_utils" +version = "0.1.0" +edition = "2021" + +# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html + +[dependencies] +rome_js_parser = {workspace = true } +rome_json_parser = {workspace = true } +rome_js_syntax = {workspace = true } +rome_json_syntax = {workspace = true } +rome_analyze = {workspace = true } +rome_rowan = {workspace = true } +rome_diagnostics = {workspace = true } +rome_service = {workspace = true } +rome_console = {workspace = true } +rome_deserialize = {workspace = true } +countme = { workspace = true, features = ["enable"] } +similar = { version = "2.2.1" } +json_comments = "0.2.1" +serde = { workspace = true } +serde_json = { workspace = true } + diff --git a/crates/rome_test_utils/src/lib.rs b/crates/rome_test_utils/src/lib.rs new file mode 100644 index 00000000000..8ad2747e73a --- /dev/null +++ b/crates/rome_test_utils/src/lib.rs @@ -0,0 +1,264 @@ +use json_comments::StripComments; +use rome_analyze::{AnalyzerAction, AnalyzerOptions}; +use rome_console::fmt::{Formatter, Termcolor}; +use rome_console::markup; +use rome_diagnostics::termcolor::Buffer; +use rome_diagnostics::{DiagnosticExt, Error, PrintDiagnostic}; +use rome_json_parser::ParseDiagnostic; +use rome_rowan::{SyntaxKind, SyntaxNode, SyntaxSlot}; +use rome_service::configuration::to_analyzer_configuration; +use rome_service::settings::{Language, WorkspaceSettings}; +use rome_service::Configuration; +use similar::TextDiff; +use std::ffi::{c_int, OsStr}; +use std::fmt::Write; +use std::path::Path; +use std::sync::Once; + +pub fn scripts_from_json(extension: &OsStr, input_code: &str) -> Option> { + if extension == "json" || extension == "jsonc" { + let input_code = StripComments::new(input_code.as_bytes()); + let scripts: Vec = serde_json::from_reader(input_code).ok()?; + Some(scripts) + } else { + None + } +} + +pub fn create_analyzer_options( + input_file: &Path, + diagnostics: &mut Vec, +) -> AnalyzerOptions { + let mut options = AnalyzerOptions::default(); + // We allow a test file to configure its rule using a special + // file with the same name as the test but with extension ".options.json" + // that configures that specific rule. + let options_file = input_file.with_extension("options.json"); + if let Ok(json) = std::fs::read_to_string(options_file.clone()) { + let deserialized = + rome_deserialize::json::deserialize_from_json_str::(json.as_str()); + if deserialized.has_errors() { + diagnostics.extend( + deserialized + .into_diagnostics() + .into_iter() + .map(|diagnostic| { + diagnostic_to_string( + options_file.file_stem().unwrap().to_str().unwrap(), + &json, + diagnostic, + ) + }) + .collect::>(), + ); + None + } else { + let configuration = deserialized.into_deserialized(); + let mut settings = WorkspaceSettings::default(); + settings.merge_with_configuration(configuration).unwrap(); + let configuration = + to_analyzer_configuration(&settings.linter, &settings.languages, |_| vec![]); + options = AnalyzerOptions { + configuration, + ..AnalyzerOptions::default() + }; + + Some(json) + } + } else { + None + }; + + options +} + +pub fn diagnostic_to_string(name: &str, source: &str, diag: Error) -> String { + let error = diag.with_file_path(name).with_file_source_code(source); + let text = markup_to_string(rome_console::markup! { + {PrintDiagnostic::verbose(&error)} + }); + + text +} + +fn markup_to_string(markup: rome_console::Markup) -> String { + let mut buffer = Vec::new(); + let mut write = + rome_console::fmt::Termcolor(rome_diagnostics::termcolor::NoColor::new(&mut buffer)); + let mut fmt = Formatter::new(&mut write); + fmt.write_markup(markup).unwrap(); + + String::from_utf8(buffer).unwrap() +} + +// Check that all red / green nodes have correctly been released on exit +extern "C" fn check_leaks() { + if let Some(report) = rome_rowan::check_live() { + panic!("\n{report}") + } +} +pub fn register_leak_checker() { + // Import the atexit function from libc + extern "C" { + fn atexit(f: extern "C" fn()) -> c_int; + } + + // Use an atomic Once to register the check_leaks function to be called + // when the process exits + static ONCE: Once = Once::new(); + ONCE.call_once(|| unsafe { + countme::enable(true); + atexit(check_leaks); + }); +} + +pub fn code_fix_to_string(source: &str, action: AnalyzerAction) -> String { + let (_, text_edit) = action.mutation.as_text_edits().unwrap_or_default(); + + let output = text_edit.new_string(source); + + let diff = TextDiff::from_lines(source, &output); + + let mut diff = diff.unified_diff(); + diff.context_radius(3); + + diff.to_string() +} + +/// The test runner for the analyzer is currently designed to have a +/// one-to-one mapping between test case and analyzer rules. +/// So each testing file will be run through the analyzer with only the rule +/// corresponding to the directory name. E.g., `style/useWhile/test.js` +/// will be analyzed with just the `style/useWhile` rule. +pub fn parse_test_path(file: &Path) -> (&str, &str) { + let rule_folder = file.parent().unwrap(); + let rule_name = rule_folder.file_name().unwrap(); + + let group_folder = rule_folder.parent().unwrap(); + let group_name = group_folder.file_name().unwrap(); + + (group_name.to_str().unwrap(), rule_name.to_str().unwrap()) +} + +/// This check is used in the parser test to ensure it doesn't emit +/// bogus nodes without diagnostics, and in the analyzer tests to +/// check the syntax trees resulting from code actions are correct +pub fn has_bogus_nodes_or_empty_slots(node: &SyntaxNode) -> bool { + node.descendants().any(|descendant| { + let kind = descendant.kind(); + if kind.is_bogus() { + return true; + } + + if kind.is_list() { + return descendant + .slots() + .any(|slot| matches!(slot, SyntaxSlot::Empty)); + } + + false + }) +} + +/// This function analyzes the parsing result of a file and panic with a +/// detailed message if it contains any error-level diagnostic, bogus nodes, +/// empty list slots or missing required children +pub fn assert_errors_are_absent( + program: &SyntaxNode, + diagnostics: &[ParseDiagnostic], + path: &Path, +) { + let debug_tree = format!("{:?}", program); + let has_missing_children = debug_tree.contains("missing (required)"); + + if diagnostics.is_empty() && !has_bogus_nodes_or_empty_slots(&program) && !has_missing_children + { + return; + } + + let mut buffer = Buffer::no_color(); + for diagnostic in diagnostics { + let error = diagnostic + .clone() + .with_file_path(path.to_str().unwrap()) + .with_file_source_code(program.to_string()); + Formatter::new(&mut Termcolor(&mut buffer)) + .write_markup(markup! { + {PrintDiagnostic::verbose(&error)} + }) + .unwrap(); + } + + panic!("There should be no errors in the file {:?} but the following errors where present:\n{}\n\nParsed tree:\n{:#?}", + path.display(), + std::str::from_utf8(buffer.as_slice()).unwrap(), + &program + ); +} + +pub fn write_analyzer_snapshot( + snapshot: &mut String, + input_code: &str, + diagnostics: &[String], + code_fixes: &[String], +) { + writeln!(snapshot, "# Input").unwrap(); + writeln!(snapshot, "```js").unwrap(); + writeln!(snapshot, "{}", input_code).unwrap(); + writeln!(snapshot, "```").unwrap(); + writeln!(snapshot).unwrap(); + + if !diagnostics.is_empty() { + writeln!(snapshot, "# Diagnostics").unwrap(); + for diagnostic in diagnostics { + writeln!(snapshot, "```").unwrap(); + writeln!(snapshot, "{}", diagnostic).unwrap(); + writeln!(snapshot, "```").unwrap(); + writeln!(snapshot).unwrap(); + } + } + + if !code_fixes.is_empty() { + writeln!(snapshot, "# Actions").unwrap(); + for action in code_fixes { + writeln!(snapshot, "```diff").unwrap(); + writeln!(snapshot, "{}", action).unwrap(); + writeln!(snapshot, "```").unwrap(); + writeln!(snapshot).unwrap(); + } + } +} + +pub fn write_transformation_snapshot( + snapshot: &mut String, + input_code: &str, + transformations: &[String], + extension: &str, +) { + writeln!(snapshot, "# Input").unwrap(); + writeln!(snapshot, "```{}", extension).unwrap(); + writeln!(snapshot, "{}", input_code).unwrap(); + writeln!(snapshot, "```").unwrap(); + writeln!(snapshot).unwrap(); + + if !transformations.is_empty() { + writeln!(snapshot, "# Transformations").unwrap(); + for transformation in transformations { + writeln!(snapshot, "```{}", extension).unwrap(); + writeln!(snapshot, "{}", transformation).unwrap(); + writeln!(snapshot, "```").unwrap(); + writeln!(snapshot).unwrap(); + } + } +} + +pub enum CheckActionType { + Suppression, + Lint, +} + +impl CheckActionType { + pub const fn is_suppression(&self) -> bool { + matches!(self, Self::Suppression) + } +} From 34ad31857c6ea382ac77bc58a505726c9f0a8391 Mon Sep 17 00:00:00 2001 From: Emanuele Stoppa Date: Fri, 21 Jul 2023 11:42:50 +0100 Subject: [PATCH 2/4] fix --- Cargo.lock | 36 ++++++++++++---- .../src/declare_transformation.rs | 2 +- .../src/transformers/ts_enum.rs | 42 ++++++++++++------- .../tests/specs/transformEnum/index.ts | 8 +++- .../tests/specs/transformEnum/index.ts.snap | 39 +++++++++++++++++ .../specs/transformEnum/index.ts.snap.new | 25 ----------- 6 files changed, 100 insertions(+), 52 deletions(-) create mode 100644 crates/rome_js_transform/tests/specs/transformEnum/index.ts.snap delete mode 100644 crates/rome_js_transform/tests/specs/transformEnum/index.ts.snap.new diff --git a/Cargo.lock b/Cargo.lock index 1d29440b4d8..b28c9f272d9 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1890,7 +1890,6 @@ dependencies = [ "bpaf", "countme", "insta", - "json_comments", "lazy_static", "natord", "roaring", @@ -1906,16 +1905,14 @@ dependencies = [ "rome_js_syntax", "rome_js_unicode_table", "rome_json_factory", - "rome_json_parser", "rome_json_syntax", "rome_rowan", - "rome_service", + "rome_test_utils", "rome_text_edit", "rustc-hash", "schemars", "serde", "serde_json", - "similar", "smallvec", "tests_macros", ] @@ -2016,14 +2013,18 @@ dependencies = [ name = "rome_js_transform" version = "0.1.0" dependencies = [ + "insta", "lazy_static", "rome_analyze", + "rome_console", "rome_diagnostics", "rome_js_factory", "rome_js_formatter", "rome_js_parser", "rome_js_syntax", "rome_rowan", + "rome_test_utils", + "tests_macros", ] [[package]] @@ -2034,21 +2035,17 @@ version = "0.0.1" name = "rome_json_analyze" version = "0.0.0" dependencies = [ - "countme", "insta", - "json_comments", "lazy_static", "rome_analyze", "rome_console", - "rome_deserialize", "rome_diagnostics", "rome_json_factory", "rome_json_parser", "rome_json_syntax", "rome_rowan", "rome_service", - "rome_text_edit", - "similar", + "rome_test_utils", "tests_macros", ] @@ -2225,6 +2222,27 @@ dependencies = [ "rome_rowan", ] +[[package]] +name = "rome_test_utils" +version = "0.1.0" +dependencies = [ + "countme", + "json_comments", + "rome_analyze", + "rome_console", + "rome_deserialize", + "rome_diagnostics", + "rome_js_parser", + "rome_js_syntax", + "rome_json_parser", + "rome_json_syntax", + "rome_rowan", + "rome_service", + "serde", + "serde_json", + "similar", +] + [[package]] name = "rome_text_edit" version = "0.0.1" diff --git a/crates/rome_js_transform/src/declare_transformation.rs b/crates/rome_js_transform/src/declare_transformation.rs index 140e4f600bd..3af2d9540e1 100644 --- a/crates/rome_js_transform/src/declare_transformation.rs +++ b/crates/rome_js_transform/src/declare_transformation.rs @@ -11,7 +11,7 @@ macro_rules! declare_transformation { impl ::rome_analyze::RuleMeta for $id { type Group = $crate::registry::TransformationGroup; const METADATA: ::rome_analyze::RuleMetadata = - ::rome_analyze::RuleMetadata::new($version, $name, concat!( $( $doc, "\n", )* )) $( .$key($value) )*; + ::rome_analyze::RuleMetadata::new($version, $name, concat!( $( $doc, "\n", )* )).recommended(true); } }; } diff --git a/crates/rome_js_transform/src/transformers/ts_enum.rs b/crates/rome_js_transform/src/transformers/ts_enum.rs index 4de4404722f..8257666e8e1 100644 --- a/crates/rome_js_transform/src/transformers/ts_enum.rs +++ b/crates/rome_js_transform/src/transformers/ts_enum.rs @@ -15,23 +15,24 @@ use rome_js_syntax::{ AnyJsAssignment, AnyJsAssignmentPattern, AnyJsBinding, AnyJsBindingPattern, AnyJsCallArgument, AnyJsExpression, AnyJsFormalParameter, AnyJsLiteralExpression, AnyJsModuleItem, AnyJsParameter, AnyJsStatement, JsAssignmentExpression, JsComputedMemberAssignment, JsExpressionStatement, - JsFunctionExpression, JsLogicalExpression, JsModule, JsModuleItemList, JsStatementList, - JsVariableStatement, TsEnumDeclaration, T, + JsFunctionExpression, JsInitializerClause, JsLogicalExpression, JsModuleItemList, + JsStatementList, JsVariableStatement, TsEnumDeclaration, T, }; -use rome_rowan::{AstNode, BatchMutationExt, TriviaPieceKind}; +use rome_rowan::{AstNode, AstSeparatedList, BatchMutationExt, TriviaPieceKind}; +use std::collections::HashMap; +use std::iter; declare_transformation! { /// Transform a TypeScript [TsEnumDeclaration] pub(crate) TsEnum { version: "next", name: "transformEnum", - recommended: false, } } pub struct TsEnumMembers { name: String, - member_names: Vec, + member_names: HashMap>, } impl Rule for TsEnum { @@ -42,12 +43,14 @@ impl Rule for TsEnum { fn run(ctx: &RuleContext) -> Self::Signals { let node = ctx.query(); - let mut member_names = vec![]; + let mut member_names = HashMap::default(); let id = node.id().ok()?; let name = id.text(); - for member in node.members() { + for (index, member) in node.members().iter().enumerate() { let member = member.ok()?; - member_names.push(member.name().ok()?.text()) + let key = member.name().ok()?.text(); + let value = member.initializer().clone(); + member_names.insert(key, value); } Some(TsEnumMembers { name, member_names }) @@ -150,9 +153,19 @@ fn make_function(node: &TsEnumMembers) -> JsFunctionExpression { fn make_members(ts_enum: &TsEnumMembers) -> JsStatementList { let mut list = vec![]; - for name in &ts_enum.member_names { + for (index, (name, value)) in ts_enum.member_names.iter().enumerate() { + let value = value + .as_ref() + .and_then(|initializer| Some(initializer.expression().ok()?.clone())) + .unwrap_or_else(|| { + AnyJsExpression::AnyJsLiteralExpression( + AnyJsLiteralExpression::JsNumberLiteralExpression( + js_number_literal_expression(ident(&index.to_string())), + ), + ) + }); list.push(AnyJsStatement::JsExpressionStatement( - make_high_order_assignment(ts_enum.name.as_str(), name.as_str(), "0"), + make_high_order_assignment(ts_enum.name.as_str(), name.as_str(), value), )); } @@ -190,7 +203,7 @@ fn make_logical_expression(node: &TsEnumMembers) -> JsLogicalExpression { fn make_high_order_assignment( enum_name: &str, member_name: &str, - member_value: &str, + member_value: AnyJsExpression, ) -> JsExpressionStatement { let left = js_computed_member_assignment( AnyJsExpression::JsIdentifierExpression(js_identifier_expression(js_reference_identifier( @@ -226,17 +239,14 @@ fn make_high_order_assignment( fn make_assignment_expression_from_member( enum_name: &str, member_name: &str, - member_value: &str, + member_value: AnyJsExpression, ) -> JsAssignmentExpression { let left = make_computed_member_assignment(enum_name, member_name); - let right = js_number_literal_expression(ident(member_value)); js_assignment_expression( AnyJsAssignmentPattern::AnyJsAssignment(AnyJsAssignment::JsComputedMemberAssignment(left)), token(T![=]), - AnyJsExpression::AnyJsLiteralExpression(AnyJsLiteralExpression::JsNumberLiteralExpression( - right, - )), + member_value, ) } diff --git a/crates/rome_js_transform/tests/specs/transformEnum/index.ts b/crates/rome_js_transform/tests/specs/transformEnum/index.ts index ea67b1d0e4d..8219135b67c 100644 --- a/crates/rome_js_transform/tests/specs/transformEnum/index.ts +++ b/crates/rome_js_transform/tests/specs/transformEnum/index.ts @@ -1,4 +1,10 @@ -enum Status { +enum StatusA { Enabled, Disabled } + + +enum StatusB { + Enabled = "Enabled", + Disabled = "Disabled" +} diff --git a/crates/rome_js_transform/tests/specs/transformEnum/index.ts.snap b/crates/rome_js_transform/tests/specs/transformEnum/index.ts.snap new file mode 100644 index 00000000000..e3a006d2600 --- /dev/null +++ b/crates/rome_js_transform/tests/specs/transformEnum/index.ts.snap @@ -0,0 +1,39 @@ +--- +source: crates/rome_js_transform/tests/spec_tests.rs +expression: index.ts +--- +# Input +```ts +enum StatusA { + Enabled, + Disabled +} + + +enum StatusB { + Enabled = "Enabled", + Disabled = "Disabled" +} + +``` + +# Transformations +```ts +var StatusA; +(function (StatusA) { + StatusA[(StatusA["Enabled"] = 0)] = "Enabled"; + StatusA[(StatusA["Disabled"] = 1)] = "Disabled"; +})(StatusA || (StatusA = {})); + +``` + +```ts +var StatusB; +(function (StatusB) { + StatusB[(StatusB["Disabled"] = "Disabled")] = "Disabled"; + StatusB[(StatusB["Enabled"] = "Enabled")] = "Enabled"; +})(StatusB || (StatusB = {})); + +``` + + diff --git a/crates/rome_js_transform/tests/specs/transformEnum/index.ts.snap.new b/crates/rome_js_transform/tests/specs/transformEnum/index.ts.snap.new deleted file mode 100644 index 0cd1a754541..00000000000 --- a/crates/rome_js_transform/tests/specs/transformEnum/index.ts.snap.new +++ /dev/null @@ -1,25 +0,0 @@ ---- -source: crates/rome_js_transform/tests/spec_tests.rs -assertion_line: 79 -expression: index.ts ---- -# Input -```ts -enum Status { - Enabled, - Disabled -} - -``` - -# Transformations -```ts -var Status; -(function (Status) { - Status[(Status["Enabled"] = 0)] = "Enabled"; - Status[(Status["Disabled"] = 0)] = "Disabled"; -})(Status || (Status = {})); - -``` - - From bbfd22bdbfed362edd0274d586e3bde3291f956a Mon Sep 17 00:00:00 2001 From: Emanuele Stoppa Date: Fri, 21 Jul 2023 12:18:36 +0100 Subject: [PATCH 3/4] feat: better transformation --- .../src/declare_transformation.rs | 2 +- crates/rome_js_transform/src/lib.rs | 17 ++--------------- .../src/transformers/ts_enum.rs | 14 ++++++-------- .../tests/specs/transformEnum/index.ts.snap | 2 +- crates/rome_test_utils/Cargo.toml | 1 + justfile | 6 ++++++ 6 files changed, 17 insertions(+), 25 deletions(-) diff --git a/crates/rome_js_transform/src/declare_transformation.rs b/crates/rome_js_transform/src/declare_transformation.rs index 3af2d9540e1..1a08db177d2 100644 --- a/crates/rome_js_transform/src/declare_transformation.rs +++ b/crates/rome_js_transform/src/declare_transformation.rs @@ -11,7 +11,7 @@ macro_rules! declare_transformation { impl ::rome_analyze::RuleMeta for $id { type Group = $crate::registry::TransformationGroup; const METADATA: ::rome_analyze::RuleMetadata = - ::rome_analyze::RuleMetadata::new($version, $name, concat!( $( $doc, "\n", )* )).recommended(true); + ::rome_analyze::RuleMetadata::new($version, $name, concat!( $( $doc, "\n", )* )); } }; } diff --git a/crates/rome_js_transform/src/lib.rs b/crates/rome_js_transform/src/lib.rs index b1905c4745d..be8604c8e3b 100644 --- a/crates/rome_js_transform/src/lib.rs +++ b/crates/rome_js_transform/src/lib.rs @@ -99,12 +99,9 @@ pub(crate) type JsBatchMutation = BatchMutation; #[cfg(test)] mod tests { - use rome_analyze::{AnalyzerOptions, Never, RuleCategories, RuleFilter, RuleKey}; - use rome_console::fmt::{Formatter, Termcolor}; - use rome_console::Markup; - use rome_diagnostics::termcolor::NoColor; + use rome_analyze::{AnalyzerOptions, Never, RuleCategories, RuleFilter}; use rome_js_parser::{parse, JsParserOptions}; - use rome_js_syntax::{JsFileSource, TextRange, TextSize}; + use rome_js_syntax::JsFileSource; use std::slice; use crate::{transform, AnalysisFilter, ControlFlow}; @@ -112,15 +109,6 @@ mod tests { #[ignore] #[test] fn quick_test() { - fn markup_to_string(markup: Markup) -> String { - let mut buffer = Vec::new(); - let mut write = Termcolor(NoColor::new(&mut buffer)); - let mut fmt = Formatter::new(&mut write); - fmt.write_markup(markup).unwrap(); - - String::from_utf8(buffer).unwrap() - } - const SOURCE: &str = r#"enum Foo { Lorem, Ipsum }"#; let parsed = parse(SOURCE, JsFileSource::tsx(), JsParserOptions::default()); @@ -139,7 +127,6 @@ mod tests { JsFileSource::tsx(), |signal| { for transformation in signal.transformations() { - dbg!(transformation.mutation.as_text_edits().unwrap_or_default()); let new_code = transformation.mutation.commit(); eprintln!("{new_code}"); } diff --git a/crates/rome_js_transform/src/transformers/ts_enum.rs b/crates/rome_js_transform/src/transformers/ts_enum.rs index 8257666e8e1..e2334233a6a 100644 --- a/crates/rome_js_transform/src/transformers/ts_enum.rs +++ b/crates/rome_js_transform/src/transformers/ts_enum.rs @@ -18,9 +18,7 @@ use rome_js_syntax::{ JsFunctionExpression, JsInitializerClause, JsLogicalExpression, JsModuleItemList, JsStatementList, JsVariableStatement, TsEnumDeclaration, T, }; -use rome_rowan::{AstNode, AstSeparatedList, BatchMutationExt, TriviaPieceKind}; -use std::collections::HashMap; -use std::iter; +use rome_rowan::{AstNode, BatchMutationExt, TriviaPieceKind}; declare_transformation! { /// Transform a TypeScript [TsEnumDeclaration] @@ -30,9 +28,10 @@ declare_transformation! { } } +#[derive(Debug)] pub struct TsEnumMembers { name: String, - member_names: HashMap>, + member_names: Vec<(String, Option)>, } impl Rule for TsEnum { @@ -43,14 +42,14 @@ impl Rule for TsEnum { fn run(ctx: &RuleContext) -> Self::Signals { let node = ctx.query(); - let mut member_names = HashMap::default(); + let mut member_names = vec![]; let id = node.id().ok()?; let name = id.text(); - for (index, member) in node.members().iter().enumerate() { + for member in node.members() { let member = member.ok()?; let key = member.name().ok()?.text(); let value = member.initializer().clone(); - member_names.insert(key, value); + member_names.push((key, value)); } Some(TsEnumMembers { name, member_names }) @@ -60,7 +59,6 @@ impl Rule for TsEnum { let node = ctx.query(); let mut mutation = node.clone().begin(); let parent = node.syntax().parent(); - if let Some(parent) = parent { if let Some(module_list) = JsModuleItemList::cast(parent) { let variable = make_variable(state); diff --git a/crates/rome_js_transform/tests/specs/transformEnum/index.ts.snap b/crates/rome_js_transform/tests/specs/transformEnum/index.ts.snap index e3a006d2600..f2f7e8bcc8e 100644 --- a/crates/rome_js_transform/tests/specs/transformEnum/index.ts.snap +++ b/crates/rome_js_transform/tests/specs/transformEnum/index.ts.snap @@ -30,8 +30,8 @@ var StatusA; ```ts var StatusB; (function (StatusB) { - StatusB[(StatusB["Disabled"] = "Disabled")] = "Disabled"; StatusB[(StatusB["Enabled"] = "Enabled")] = "Enabled"; + StatusB[(StatusB["Disabled"] = "Disabled")] = "Disabled"; })(StatusB || (StatusB = {})); ``` diff --git a/crates/rome_test_utils/Cargo.toml b/crates/rome_test_utils/Cargo.toml index 8a220503fa8..7d24ffa7e73 100644 --- a/crates/rome_test_utils/Cargo.toml +++ b/crates/rome_test_utils/Cargo.toml @@ -2,6 +2,7 @@ name = "rome_test_utils" version = "0.1.0" edition = "2021" +publish = false # See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html diff --git a/justfile b/justfile index d3ddd54e3a3..fd5ba046098 100644 --- a/justfile +++ b/justfile @@ -90,6 +90,12 @@ test-lintrule name: cargo test -p rome_js_analyze -- {{snakecase(name)}} cargo test -p rome_json_analyze -- {{snakecase(name)}} +# Tests a lint rule. The name of the rule needs to be camel case +test-transformation name: + just _touch crates/rome_js_transform/tests/spec_tests.rs + cargo test -p rome_js_transform -- {{snakecase(name)}} + + # Alias for `cargo lint`, it runs clippy on the whole codebase lint: cargo lint From de0bcb4b3bf07d156616bbaa2b4c0777f2fa5162 Mon Sep 17 00:00:00 2001 From: Emanuele Stoppa Date: Sun, 23 Jul 2023 11:59:02 +0100 Subject: [PATCH 4/4] clippy --- crates/rome_js_analyze/tests/spec_tests.rs | 2 +- crates/rome_js_syntax/src/file_source.rs | 2 +- crates/rome_js_transform/src/transformers/ts_enum.rs | 2 +- crates/rome_js_transform/tests/spec_tests.rs | 4 ++-- crates/rome_json_analyze/tests/spec_tests.rs | 2 +- crates/rome_rowan/src/ast/batch.rs | 2 +- crates/rome_test_utils/src/lib.rs | 3 +-- 7 files changed, 8 insertions(+), 9 deletions(-) diff --git a/crates/rome_js_analyze/tests/spec_tests.rs b/crates/rome_js_analyze/tests/spec_tests.rs index f4ea2ff00a8..2e456cfd1d2 100644 --- a/crates/rome_js_analyze/tests/spec_tests.rs +++ b/crates/rome_js_analyze/tests/spec_tests.rs @@ -206,7 +206,7 @@ fn check_code_action( // Re-parse the modified code and panic if the resulting tree has syntax errors let re_parse = parse(&output, source_type, options); - assert_errors_are_absent(&re_parse.tree().syntax(), re_parse.diagnostics(), path); + assert_errors_are_absent(re_parse.tree().syntax(), re_parse.diagnostics(), path); } pub(crate) fn run_suppression_test(input: &'static str, _: &str, _: &str, _: &str) { diff --git a/crates/rome_js_syntax/src/file_source.rs b/crates/rome_js_syntax/src/file_source.rs index 058d0a0cdbd..b29a2f4433c 100644 --- a/crates/rome_js_syntax/src/file_source.rs +++ b/crates/rome_js_syntax/src/file_source.rs @@ -182,7 +182,7 @@ impl JsFileSource { self.module_kind.is_module() } - pub fn as_extension_name(&self) -> &str { + pub fn file_extension(&self) -> &str { match self.language { Language::JavaScript => { if matches!(self.variant, LanguageVariant::Jsx) { diff --git a/crates/rome_js_transform/src/transformers/ts_enum.rs b/crates/rome_js_transform/src/transformers/ts_enum.rs index e2334233a6a..e124b898261 100644 --- a/crates/rome_js_transform/src/transformers/ts_enum.rs +++ b/crates/rome_js_transform/src/transformers/ts_enum.rs @@ -154,7 +154,7 @@ fn make_members(ts_enum: &TsEnumMembers) -> JsStatementList { for (index, (name, value)) in ts_enum.member_names.iter().enumerate() { let value = value .as_ref() - .and_then(|initializer| Some(initializer.expression().ok()?.clone())) + .and_then(|initializer| initializer.expression().ok()) .unwrap_or_else(|| { AnyJsExpression::AnyJsLiteralExpression( AnyJsLiteralExpression::JsNumberLiteralExpression( diff --git a/crates/rome_js_transform/tests/spec_tests.rs b/crates/rome_js_transform/tests/spec_tests.rs index fc789942b5f..5ed6af18988 100644 --- a/crates/rome_js_transform/tests/spec_tests.rs +++ b/crates/rome_js_transform/tests/spec_tests.rs @@ -127,7 +127,7 @@ pub(crate) fn analyze_and_snap( snapshot, input_code, transformations.as_slice(), - source_type.as_extension_name(), + source_type.file_extension(), ); diagnostics.len() @@ -164,5 +164,5 @@ fn check_transformation( // Re-parse the modified code and panic if the resulting tree has syntax errors let re_parse = parse(&output, source_type, options); - assert_errors_are_absent(&re_parse.tree().syntax(), re_parse.diagnostics(), path); + assert_errors_are_absent(re_parse.tree().syntax(), re_parse.diagnostics(), path); } diff --git a/crates/rome_json_analyze/tests/spec_tests.rs b/crates/rome_json_analyze/tests/spec_tests.rs index 85808895577..8bed06e2c41 100644 --- a/crates/rome_json_analyze/tests/spec_tests.rs +++ b/crates/rome_json_analyze/tests/spec_tests.rs @@ -1,7 +1,7 @@ use rome_analyze::{AnalysisFilter, AnalyzerAction, ControlFlow, Never, RuleFilter}; use rome_diagnostics::advice::CodeSuggestionAdvice; use rome_diagnostics::{DiagnosticExt, Severity}; -use rome_json_parser::parse_json; +use rome_json_parser::{parse_json, JsonParserOptions}; use rome_json_syntax::JsonLanguage; use rome_rowan::AstNode; use rome_test_utils::{ diff --git a/crates/rome_rowan/src/ast/batch.rs b/crates/rome_rowan/src/ast/batch.rs index bed67aebd85..09da8b6fbea 100644 --- a/crates/rome_rowan/src/ast/batch.rs +++ b/crates/rome_rowan/src/ast/batch.rs @@ -310,7 +310,7 @@ where for change in &self.changes { let parent = change.parent.as_ref().unwrap_or(&self.root); - let delete = match dbg!(parent.slots().nth(change.new_node_slot)) { + let delete = match parent.slots().nth(change.new_node_slot) { Some(SyntaxSlot::Node(node)) => node.text_range(), Some(SyntaxSlot::Token(token)) => token.text_range(), _ => continue, diff --git a/crates/rome_test_utils/src/lib.rs b/crates/rome_test_utils/src/lib.rs index 8ad2747e73a..0995286b086 100644 --- a/crates/rome_test_utils/src/lib.rs +++ b/crates/rome_test_utils/src/lib.rs @@ -171,8 +171,7 @@ pub fn assert_errors_are_absent( let debug_tree = format!("{:?}", program); let has_missing_children = debug_tree.contains("missing (required)"); - if diagnostics.is_empty() && !has_bogus_nodes_or_empty_slots(&program) && !has_missing_children - { + if diagnostics.is_empty() && !has_bogus_nodes_or_empty_slots(program) && !has_missing_children { return; }