From 117ae36775f96914cf540bca7ebab3a104d5957f Mon Sep 17 00:00:00 2001 From: Boshen <1430279+Boshen@users.noreply.github.com> Date: Wed, 14 Aug 2024 11:11:13 +0000 Subject: [PATCH] refactor(coverage): add driver struct for adding common checks later (#4893) --- tasks/coverage/src/driver.rs | 167 ++++++++++++++++++ tasks/coverage/src/lib.rs | 2 + tasks/coverage/src/suite.rs | 88 ++------- tasks/coverage/src/test262/mod.rs | 33 ---- tasks/coverage/src/tools/codegen.rs | 46 ++--- tasks/coverage/src/tools/minifier.rs | 32 ++-- tasks/coverage/src/tools/transformer.rs | 49 ++--- .../src/typescript/transpile_runner.rs | 8 +- 8 files changed, 230 insertions(+), 195 deletions(-) create mode 100644 tasks/coverage/src/driver.rs diff --git a/tasks/coverage/src/driver.rs b/tasks/coverage/src/driver.rs new file mode 100644 index 0000000000000..2c12c1c7c4e01 --- /dev/null +++ b/tasks/coverage/src/driver.rs @@ -0,0 +1,167 @@ +use std::collections::HashSet; +use std::path::PathBuf; + +use oxc_allocator::Allocator; +use oxc_ast::Trivias; +use oxc_codegen::{CodeGenerator, CommentOptions, WhitespaceRemover}; +use oxc_diagnostics::OxcDiagnostic; +use oxc_minifier::{CompressOptions, Compressor}; +use oxc_parser::{Parser, ParserReturn}; +use oxc_semantic::{Semantic, SemanticBuilder}; +use oxc_span::{SourceType, Span}; +use oxc_transformer::{TransformOptions, Transformer}; + +use crate::suite::TestResult; + +#[allow(clippy::struct_excessive_bools)] +#[derive(Default)] +pub struct Driver { + pub path: PathBuf, + // options + pub transform: Option, + pub compress: bool, + pub remove_whitespace: bool, + pub codegen: bool, + pub allow_return_outside_function: bool, + // results + pub panicked: bool, + pub errors: Vec, + pub printed: String, +} + +impl Driver { + pub fn errors(&mut self) -> Vec { + std::mem::take(&mut self.errors) + } + + pub fn idempotency( + mut self, + case: &'static str, + source_text: &str, + source_type: SourceType, + ) -> TestResult { + self.run(source_text, source_type); + let printed1 = self.printed.clone(); + self.run(&printed1, source_type); + let printed2 = self.printed.clone(); + if printed1 == printed2 { + TestResult::Passed + } else { + TestResult::Mismatch(case, printed1, printed2) + } + } + + pub fn run(&mut self, source_text: &str, source_type: SourceType) { + let allocator = Allocator::default(); + let ParserReturn { mut program, errors, trivias, panicked } = + Parser::new(&allocator, source_text, source_type) + .allow_return_outside_function(self.allow_return_outside_function) + .parse(); + self.panicked = panicked; + + if self.check_comments(&trivias) { + return; + } + + // Make sure serialization doesn't crash; also for code coverage. + let _serializer = program.serializer(); + + if !errors.is_empty() { + self.errors.extend(errors); + } + + let semantic_ret = SemanticBuilder::new(source_text, source_type) + .with_trivias(trivias.clone()) + .with_check_syntax_error(true) + .build_module_record(self.path.clone(), &program) + .build(&program); + + if !semantic_ret.errors.is_empty() { + self.errors.extend(semantic_ret.errors); + return; + } + + // TODO + // if self.check_semantic(&semantic_ret.semantic) { + // return; + // } + + if let Some(options) = self.transform.clone() { + Transformer::new( + &allocator, + &self.path, + source_type, + source_text, + trivias.clone(), + options, + ) + .build(&mut program); + } + + if self.compress { + Compressor::new(&allocator, CompressOptions::all_true()).build(&mut program); + } + + if self.codegen { + let comment_options = CommentOptions { preserve_annotate_comments: true }; + + let printed = if self.remove_whitespace { + WhitespaceRemover::new().build(&program).source_text + } else { + CodeGenerator::new() + .enable_comment(source_text, trivias, comment_options) + .build(&program) + .source_text + }; + + self.printed = printed; + } + } + + fn check_comments(&mut self, trivias: &Trivias) -> bool { + let mut uniq: HashSet = HashSet::new(); + for comment in trivias.comments() { + if !uniq.insert(comment.span) { + self.errors + .push(OxcDiagnostic::error("Duplicate Comment").with_label(comment.span)); + return true; + } + } + false + } + + #[allow(unused)] + fn check_semantic(&mut self, semantic: &Semantic<'_>) -> bool { + if are_all_identifiers_resolved(semantic) { + return false; + } + self.errors.push(OxcDiagnostic::error("symbol or reference is not set")); + false + } +} + +#[allow(unused)] +fn are_all_identifiers_resolved(semantic: &Semantic<'_>) -> bool { + use oxc_ast::AstKind; + use oxc_semantic::AstNode; + + let ast_nodes = semantic.nodes(); + let has_non_resolved = ast_nodes.iter().any(|node| { + match node.kind() { + AstKind::BindingIdentifier(id) => { + let mut parents = ast_nodes.iter_parents(node.id()).map(AstNode::kind); + parents.next(); // Exclude BindingIdentifier itself + if let (Some(AstKind::Function(_)), Some(AstKind::IfStatement(_))) = + (parents.next(), parents.next()) + { + return false; + } + id.symbol_id.get().is_none() + } + AstKind::IdentifierReference(ref_id) => ref_id.reference_id.get().is_none(), + _ => false, + } + }); + + !has_non_resolved +} diff --git a/tasks/coverage/src/lib.rs b/tasks/coverage/src/lib.rs index 09e3fc9a6d8ef..96b49791b81d2 100644 --- a/tasks/coverage/src/lib.rs +++ b/tasks/coverage/src/lib.rs @@ -8,6 +8,7 @@ mod misc; mod test262; mod typescript; +mod driver; mod tools; use std::{fs, path::PathBuf, process::Command, time::Duration}; @@ -18,6 +19,7 @@ use similar::DiffableStr; use crate::{ babel::{BabelCase, BabelSuite}, + driver::Driver, misc::{MiscCase, MiscSuite}, suite::Suite, test262::{Test262Case, Test262Suite}, diff --git a/tasks/coverage/src/suite.rs b/tasks/coverage/src/suite.rs index d8eb51b2b5242..8d9936cfe24bc 100644 --- a/tasks/coverage/src/suite.rs +++ b/tasks/coverage/src/suite.rs @@ -1,5 +1,4 @@ use std::{ - collections::HashSet, fs, io::{stdout, Read, Write}, panic::UnwindSafe, @@ -11,33 +10,25 @@ use console::Style; use encoding_rs::UTF_16LE; use encoding_rs_io::DecodeReaderBytesBuilder; use futures::future::join_all; -use oxc_allocator::Allocator; -use oxc_ast::Trivias; use oxc_diagnostics::{GraphicalReportHandler, GraphicalTheme, NamedSource}; -use oxc_parser::Parser; -use oxc_semantic::SemanticBuilder; -use oxc_span::{SourceType, Span}; +use oxc_span::SourceType; use oxc_tasks_common::{normalize_path, Snapshot}; use rayon::prelude::*; use similar::{ChangeTag, TextDiff}; use tokio::runtime::Runtime; use walkdir::WalkDir; -use crate::{project_root, AppArgs}; +use crate::{project_root, AppArgs, Driver}; #[derive(Debug, PartialEq)] pub enum TestResult { ToBeRun, Passed, IncorrectlyPassed, - #[allow(unused)] - // (actual, expected) - Mismatch(String, String), + Mismatch(/* case */ &'static str, /* actual */ String, /* expected */ String), ParseError(String, /* panicked */ bool), CorrectError(String, /* panicked */ bool), RuntimeError(String), - CodegenError(/* reason */ &'static str), - DuplicatedComments(String), Snapshot(String), } @@ -318,28 +309,16 @@ pub trait Case: Sized + Sync + Send + UnwindSafe { /// Execute the parser once and get the test result fn execute(&mut self, source_type: SourceType) -> TestResult { - let allocator = Allocator::default(); let source_text = self.code(); - let parser_ret = Parser::new(&allocator, source_text, source_type) - .allow_return_outside_function(self.allow_return_outside_function()) - .parse(); - if let Some(res) = self.check_comments(&parser_ret.trivias) { - return res; - } + let path = self.path(); - // Make sure serialization doesn't crash; also for code coverage. - let _serializer = parser_ret.program.serializer(); - - let program = allocator.alloc(parser_ret.program); - let semantic_ret = SemanticBuilder::new(source_text, source_type) - .with_trivias(parser_ret.trivias) - .with_check_syntax_error(true) - .build_module_record(PathBuf::new(), program) - .build(program); - if let Some(res) = self.check_semantic(&semantic_ret.semantic) { - return res; - } - let errors = parser_ret.errors.into_iter().chain(semantic_ret.errors).collect::>(); + let mut driver = Driver { + path: path.to_path_buf(), + allow_return_outside_function: self.allow_return_outside_function(), + ..Driver::default() + }; + driver.run(source_text, source_type); + let errors = driver.errors(); let result = if errors.is_empty() { Ok(String::new()) @@ -349,7 +328,7 @@ pub trait Case: Sized + Sync + Send + UnwindSafe { let mut output = String::new(); for error in errors { let error = error.with_source_code(NamedSource::new( - normalize_path(self.path()), + normalize_path(path), source_text.to_string(), )); handler.render_report(&mut output, error.as_ref()).unwrap(); @@ -359,8 +338,8 @@ pub trait Case: Sized + Sync + Send + UnwindSafe { let should_fail = self.should_fail(); match result { - Err(err) if should_fail => TestResult::CorrectError(err, parser_ret.panicked), - Err(err) if !should_fail => TestResult::ParseError(err, parser_ret.panicked), + Err(err) if should_fail => TestResult::CorrectError(err, driver.panicked), + Err(err) if !should_fail => TestResult::ParseError(err, driver.panicked), Ok(_) if should_fail => TestResult::IncorrectlyPassed, Ok(_) if !should_fail => TestResult::Passed, _ => unreachable!(), @@ -375,13 +354,12 @@ pub trait Case: Sized + Sync + Send + UnwindSafe { )?; writer.write_all(error.as_bytes())?; } - TestResult::Mismatch(ast_string, expected_ast_string) => { - writer.write_all( - format!("Mismatch: {:?}\n", normalize_path(self.path())).as_bytes(), - )?; + TestResult::Mismatch(case, ast_string, expected_ast_string) => { + writer + .write_all(format!("{case}: {:?}\n", normalize_path(self.path())).as_bytes())?; if args.diff { self.print_diff(writer, ast_string.as_str(), expected_ast_string.as_str())?; - println!("Mismatch: {:?}", normalize_path(self.path())); + println!("{case}: {:?}", normalize_path(self.path())); } } TestResult::RuntimeError(error) => { @@ -396,23 +374,9 @@ pub trait Case: Sized + Sync + Send + UnwindSafe { format!("Expect Syntax Error: {:?}\n", normalize_path(self.path())).as_bytes(), )?; } - TestResult::CodegenError(reason) => { - writer.write_all( - format!("{reason} failed: {:?}\n", normalize_path(self.path())).as_bytes(), - )?; - } TestResult::Snapshot(snapshot) => { writer.write_all(snapshot.as_bytes())?; } - TestResult::DuplicatedComments(comment) => { - writer.write_all( - format!( - "Duplicated comments \"{comment}\": {:?}\n", - normalize_path(self.path()) - ) - .as_bytes(), - )?; - } TestResult::Passed | TestResult::ToBeRun | TestResult::CorrectError(..) => {} } Ok(()) @@ -437,20 +401,4 @@ pub trait Case: Sized + Sync + Send + UnwindSafe { } Ok(()) } - - fn check_semantic(&self, _semantic: &oxc_semantic::Semantic<'_>) -> Option { - None - } - - fn check_comments(&self, trivias: &Trivias) -> Option { - let mut uniq: HashSet = HashSet::new(); - for comment in trivias.comments() { - if !uniq.insert(comment.span) { - return Some(TestResult::DuplicatedComments( - comment.span.source_text(self.code()).to_string(), - )); - } - } - None - } } diff --git a/tasks/coverage/src/test262/mod.rs b/tasks/coverage/src/test262/mod.rs index cb13cfe755ae5..b8ca968edfd56 100644 --- a/tasks/coverage/src/test262/mod.rs +++ b/tasks/coverage/src/test262/mod.rs @@ -137,37 +137,4 @@ impl Case for Test262Case { } }; } - - fn check_semantic(&self, semantic: &oxc_semantic::Semantic<'_>) -> Option { - if are_all_identifiers_resolved(semantic) { - None - } else { - Some(TestResult::ParseError("Unset symbol / reference".to_string(), true)) - } - } -} - -fn are_all_identifiers_resolved(semantic: &oxc_semantic::Semantic<'_>) -> bool { - use oxc_ast::AstKind; - use oxc_semantic::AstNode; - - let ast_nodes = semantic.nodes(); - let has_non_resolved = ast_nodes.iter().any(|node| { - match node.kind() { - AstKind::BindingIdentifier(id) => { - let mut parents = ast_nodes.iter_parents(node.id()).map(AstNode::kind); - parents.next(); // Exclude BindingIdentifier itself - if let (Some(AstKind::Function(_)), Some(AstKind::IfStatement(_))) = - (parents.next(), parents.next()) - { - return false; - } - id.symbol_id.get().is_none() - } - AstKind::IdentifierReference(ref_id) => ref_id.reference_id.get().is_none(), - _ => false, - } - }); - - !has_non_resolved } diff --git a/tasks/coverage/src/tools/codegen.rs b/tasks/coverage/src/tools/codegen.rs index 7aa4b3b1c48c7..83a240fc869e4 100644 --- a/tasks/coverage/src/tools/codegen.rs +++ b/tasks/coverage/src/tools/codegen.rs @@ -1,8 +1,5 @@ use std::path::{Path, PathBuf}; -use oxc_allocator::Allocator; -use oxc_codegen::{CodeGenerator, WhitespaceRemover}; -use oxc_parser::Parser; use oxc_span::SourceType; use crate::{ @@ -11,46 +8,29 @@ use crate::{ suite::{Case, TestResult}, test262::{Test262Case, TestFlag}, typescript::TypeScriptCase, + Driver, }; +/// Idempotency test fn get_result(source_text: &str, source_type: SourceType) -> TestResult { - let normal_result = get_normal_result(source_text, source_type); - if !normal_result { - return TestResult::CodegenError("Normal"); + let result = Driver { codegen: true, ..Driver::default() }.idempotency( + "Normal", + source_text, + source_type, + ); + if result != TestResult::Passed { + return result; }; - let minify_result = get_minify_result(source_text, source_type); - if !minify_result { - return TestResult::CodegenError("Minify"); + let result = Driver { codegen: true, remove_whitespace: true, ..Driver::default() } + .idempotency("Minify", source_text, source_type); + if result != TestResult::Passed { + return result; } TestResult::Passed } -/// Idempotency test -fn get_normal_result(source_text: &str, source_type: SourceType) -> bool { - let allocator = Allocator::default(); - let source_text1 = { - let ret = Parser::new(&allocator, source_text, source_type).parse(); - CodeGenerator::new().build(&ret.program).source_text - }; - let source_text2 = { - let ret = Parser::new(&allocator, &source_text1, source_type).parse(); - CodeGenerator::new().build(&ret.program).source_text - }; - source_text1 == source_text2 -} - -/// Minify idempotency test -fn get_minify_result(source_text: &str, source_type: SourceType) -> bool { - let allocator = Allocator::default(); - let parse_result1 = Parser::new(&allocator, source_text, source_type).parse(); - let source_text1 = WhitespaceRemover::new().build(&parse_result1.program).source_text; - let parse_result2 = Parser::new(&allocator, source_text1.as_str(), source_type).parse(); - let source_text2 = WhitespaceRemover::new().build(&parse_result2.program).source_text; - source_text1 == source_text2 -} - pub struct CodegenTest262Case { base: Test262Case, } diff --git a/tasks/coverage/src/tools/minifier.rs b/tasks/coverage/src/tools/minifier.rs index 81ac268322e74..da8d16304290f 100644 --- a/tasks/coverage/src/tools/minifier.rs +++ b/tasks/coverage/src/tools/minifier.rs @@ -1,17 +1,23 @@ use std::path::{Path, PathBuf}; -use oxc_allocator::Allocator; -use oxc_codegen::CodeGenerator; -use oxc_minifier::{CompressOptions, Compressor}; -use oxc_parser::Parser; use oxc_span::SourceType; use crate::{ babel::BabelCase, suite::{Case, TestResult}, test262::{Test262Case, TestFlag}, + Driver, }; +/// Idempotency test +fn get_result(source_text: &str, source_type: SourceType) -> TestResult { + Driver { compress: true, codegen: true, ..Driver::default() }.idempotency( + "Compress", + source_text, + source_type, + ) +} + pub struct MinifierTest262Case { base: Test262Case, } @@ -80,21 +86,3 @@ impl Case for MinifierBabelCase { self.base.set_result(result); } } -// Test minification by minifying twice because it is a idempotent -fn get_result(source_text: &str, source_type: SourceType) -> TestResult { - let source_text1 = minify(source_text, source_type); - let source_text2 = minify(&source_text1, source_type); - if source_text1 == source_text2 { - TestResult::Passed - } else { - TestResult::ParseError(String::new(), false) - } -} - -fn minify(source_text: &str, source_type: SourceType) -> String { - let allocator = Allocator::default(); - let ret = Parser::new(&allocator, source_text, source_type).parse(); - let program = allocator.alloc(ret.program); - Compressor::new(&allocator, CompressOptions::all_true()).build(program); - CodeGenerator::new().build(program).source_text -} diff --git a/tasks/coverage/src/tools/transformer.rs b/tasks/coverage/src/tools/transformer.rs index 617f6e1ee5c52..06069d886239a 100644 --- a/tasks/coverage/src/tools/transformer.rs +++ b/tasks/coverage/src/tools/transformer.rs @@ -1,16 +1,14 @@ use std::path::{Path, PathBuf}; -use oxc_allocator::Allocator; -use oxc_codegen::CodeGenerator; -use oxc_parser::Parser; use oxc_span::SourceType; use oxc_transformer::{ ArrowFunctionsOptions, ES2015Options, ReactJsxRuntime, ReactOptions, TransformOptions, - Transformer, TypeScriptOptions, + TypeScriptOptions, }; use crate::{ babel::BabelCase, + driver::Driver, misc::MiscCase, suite::{Case, TestResult}, test262::{Test262Case, TestFlag}, @@ -24,44 +22,25 @@ fn get_result( source_path: &Path, options: Option, ) -> TestResult { - let allocator = Allocator::default(); - let options = options.unwrap_or_else(get_default_transformer_options); - - // First pass + let mut driver = Driver { + path: source_path.to_path_buf(), + transform: Some(options.unwrap_or_else(get_default_transformer_options)), + codegen: true, + ..Driver::default() + }; let transformed1 = { - let mut ret1 = Parser::new(&allocator, source_text, source_type).parse(); - let _ = Transformer::new( - &allocator, - source_path, - source_type, - source_text, - ret1.trivias.clone(), - options.clone(), - ) - .build(&mut ret1.program); - CodeGenerator::new().build(&ret1.program).source_text + driver.run(source_text, source_type); + driver.printed.clone() }; - - // Second pass with only JavaScript parsing + // Second pass with only JavaScript syntax let transformed2 = { - let source_type = SourceType::default().with_module(source_type.is_module()); - let mut ret2 = Parser::new(&allocator, &transformed1, source_type).parse(); - let _ = Transformer::new( - &allocator, - source_path, - source_type, - &transformed1, - ret2.trivias.clone(), - options, - ) - .build(&mut ret2.program); - CodeGenerator::new().build(&ret2.program).source_text + driver.run(&transformed1, SourceType::default().with_module(source_type.is_module())); + driver.printed.clone() }; - if transformed1 == transformed2 { TestResult::Passed } else { - TestResult::Mismatch(transformed1, transformed2) + TestResult::Mismatch("Mismatch", transformed1, transformed2) } } diff --git a/tasks/coverage/src/typescript/transpile_runner.rs b/tasks/coverage/src/typescript/transpile_runner.rs index 633b547af1c15..f7cd87fe369f4 100644 --- a/tasks/coverage/src/typescript/transpile_runner.rs +++ b/tasks/coverage/src/typescript/transpile_runner.rs @@ -109,13 +109,17 @@ impl TypeScriptTranspileCase { let baseline_text = baseline.print(); if expected.files.len() != baseline.files.len() { - return TestResult::Mismatch(baseline_text, expected_text); + return TestResult::Mismatch("Mismatch", baseline_text, expected_text); } for (base, expected) in baseline.files.iter().zip(expected.files) { if expected.original_diagnostic.is_empty() { if base.oxc_printed != expected.oxc_printed { - return TestResult::Mismatch(base.oxc_printed.clone(), expected.oxc_printed); + return TestResult::Mismatch( + "Mismatch", + base.oxc_printed.clone(), + expected.oxc_printed, + ); } } else { let matched = base.oxc_diagnostics.iter().zip(expected.original_diagnostic).all(