From c956f7e89726b3f9fdc3308135a93f86232931aa Mon Sep 17 00:00:00 2001 From: Boshen Date: Mon, 31 Jul 2023 12:40:14 +0800 Subject: [PATCH] refactor(cli): remove experimental code "module_tree_handler" (#670) This code will eventually be superseded by https://github.com/web-infra-dev/oxc/pull/530, removing this for now so others don't have to touch this. --- Cargo.lock | 1 - crates/oxc_cli/Cargo.toml | 1 - crates/oxc_cli/src/lint/mod.rs | 18 +- .../oxc_cli/src/lint/module_tree_handler.rs | 296 ------------------ crates/oxc_cli/src/lint/options.rs | 4 +- 5 files changed, 4 insertions(+), 316 deletions(-) delete mode 100644 crates/oxc_cli/src/lint/module_tree_handler.rs diff --git a/Cargo.lock b/Cargo.lock index a49218aeb..7f26eb9fc 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1366,7 +1366,6 @@ version = "0.0.0" dependencies = [ "clap", "codespan-reporting", - "crossbeam-channel", "dashmap", "ignore", "jemallocator", diff --git a/crates/oxc_cli/Cargo.toml b/crates/oxc_cli/Cargo.toml index 83eb29628..ceb5ce069 100644 --- a/crates/oxc_cli/Cargo.toml +++ b/crates/oxc_cli/Cargo.toml @@ -36,7 +36,6 @@ oxc_type_synthesis = { workspace = true } codespan-reporting = "0.11.1" clap = { workspace = true } -crossbeam-channel = { workspace = true } dashmap = { workspace = true } ignore = { workspace = true, features = ["simd-accel"] } miette = { workspace = true, features = ["fancy-no-backtrace"] } diff --git a/crates/oxc_cli/src/lint/mod.rs b/crates/oxc_cli/src/lint/mod.rs index ec09d2276..8a7cc3afa 100644 --- a/crates/oxc_cli/src/lint/mod.rs +++ b/crates/oxc_cli/src/lint/mod.rs @@ -1,7 +1,6 @@ mod command; mod error; mod isolated_handler; -mod module_tree_handler; mod options; use std::{io::BufWriter, sync::Arc, time::Duration}; @@ -11,10 +10,7 @@ use oxc_linter::{Linter, RuleCategory, RuleEnum, RULES}; use rustc_hash::FxHashSet; pub use self::{error::Error, options::LintOptions}; -use self::{ - isolated_handler::IsolatedLintHandler, module_tree_handler::ModuleTreeLintHandler, - options::AllowWarnDeny, -}; +use self::{isolated_handler::IsolatedLintHandler, options::AllowWarnDeny}; use crate::{CliRunResult, Runner}; pub struct LintRunner { @@ -48,11 +44,8 @@ impl Runner for LintRunner { return CliRunResult::None; } - let result = if Self::enable_module_tree() { - ModuleTreeLintHandler::new(Arc::clone(&self.options), Arc::clone(&self.linter)).run() - } else { - IsolatedLintHandler::new(Arc::clone(&self.options), Arc::clone(&self.linter)).run() - }; + let result = + IsolatedLintHandler::new(Arc::clone(&self.options), Arc::clone(&self.linter)).run(); if self.options.print_execution_times { self.print_execution_times(); @@ -63,11 +56,6 @@ impl Runner for LintRunner { } impl LintRunner { - /// check if the module tree should be provided when linting - fn enable_module_tree() -> bool { - matches!(std::env::var("OXC_MODULE_TREE"), Ok(x) if x == "true" || x == "1") - } - fn print_rules() { let mut stdout = BufWriter::new(std::io::stdout()); Linter::print_rules(&mut stdout); diff --git a/crates/oxc_cli/src/lint/module_tree_handler.rs b/crates/oxc_cli/src/lint/module_tree_handler.rs deleted file mode 100644 index f044d6366..000000000 --- a/crates/oxc_cli/src/lint/module_tree_handler.rs +++ /dev/null @@ -1,296 +0,0 @@ -use std::{ - fs, - io::{self, BufWriter, Write}, - path::{Path, PathBuf}, - rc::Rc, - sync::Arc, -}; - -use crossbeam_channel::{unbounded, Receiver, Sender}; -use miette::NamedSource; -use oxc_allocator::Allocator; -use oxc_diagnostics::{ - miette::{self, Diagnostic}, - thiserror::Error, - Error, GraphicalReportHandler, Severity, -}; -use oxc_linter::{FixResult, Fixer, LintContext, Linter}; -use oxc_parser::{Parser, ParserReturn}; -use oxc_semantic::{SemanticBuilder, SemanticBuilderReturn}; -use oxc_span::SourceType; -use rayon::prelude::*; - -use super::{ - error::{ErrorWithPath, Result}, - options::LintOptions, -}; -use crate::CliRunResult; - -#[derive(Clone)] -struct LinterRuntimeData { - linter: Arc, - visited: Arc>, - tx_error: Sender<(PathBuf, Vec)>, -} - -pub struct ModuleTreeLintHandler { - options: Arc, - linter: Arc, -} -use dashmap::DashSet; - -#[derive(Debug, Error, Diagnostic)] -#[error("File is too long to fit on the screen")] -#[diagnostic(help("{0:?} seems like a minified file"))] -pub struct MinifiedFileError(pub PathBuf); - -impl ModuleTreeLintHandler { - pub(super) fn new(options: Arc, linter: Arc) -> Self { - Self { options, linter } - } - - pub(super) fn run(&self) -> CliRunResult { - let now = std::time::Instant::now(); - - // Unless other panic happens, calling `Sender::send` can't fail, because we hold the - // receiver until all senders are dropped. This allows us to safely unwrap all calls to send. - let (tx_error, rx_error) = unbounded(); - - // we can ignore the result because nothing bad happens if the resolver is already set - // TODO: make sure this is still true once we allow options to be set - // during runtime (config file, args, etc.) - // let _ = RESOLVER.set(Resolver::default()); - - let visited = Arc::new(DashSet::new()); - - // TODO: try to process as many files as possible even if some of them fail - let result = process_paths( - &self.options.paths, - LinterRuntimeData { - linter: Arc::clone(&self.linter), - visited: Arc::clone(&visited), - tx_error, - }, - ); - - let (number_of_warnings, number_of_errors) = self.process_diagnostics(&rx_error); - - if let Err(err) = result { - return CliRunResult::IOError(err); - } - - CliRunResult::LintResult { - duration: now.elapsed(), - number_of_rules: self.linter.number_of_rules(), - number_of_files: visited.len(), - number_of_warnings, - number_of_errors, - max_warnings_exceeded: self - .options - .max_warnings - .map_or(false, |max_warnings| number_of_warnings > max_warnings), - } - } - - fn process_diagnostics(&self, rx_error: &Receiver<(PathBuf, Vec)>) -> (usize, usize) { - let mut number_of_warnings = 0; - let mut number_of_errors = 0; - let mut buf_writer = BufWriter::new(io::stdout()); - let handler = GraphicalReportHandler::new(); - - for (path, diagnostics) in rx_error { - let mut output = String::new(); - for diagnostic in diagnostics { - let severity = diagnostic.severity(); - let is_warning = severity == Some(Severity::Warning); - let is_error = severity.is_none() || severity == Some(Severity::Error); - if is_warning || is_error { - if is_warning { - number_of_warnings += 1; - } - if is_error { - number_of_errors += 1; - } - // The --quiet flag follows ESLint's --quiet behavior as documented here: https://eslint.org/docs/latest/use/command-line-interface#--quiet - // Note that it does not disable ALL diagnostics, only Warning diagnostics - if self.options.quiet { - continue; - } - - if let Some(max_warnings) = self.options.max_warnings { - if number_of_warnings > max_warnings { - continue; - } - } - } - - let mut err = String::new(); - handler - .render_report(&mut err, diagnostic.as_ref()) - .expect("Writing to a string can't fail"); - - if err.lines().all(|line| line.len() < 400) { - output.push_str(&err); - continue; - } - - // If the error is too long, we assume it's a minified file and print it as only error - output = format!("{:?}", Error::new(MinifiedFileError(path.clone()))); - break; - } - - // write operations on stdout can't fail according to RFC 1014 - // https://rust-lang.github.io/rfcs/1014-stdout-existential-crisis.html - buf_writer.write_all(output.as_bytes()).expect("Writing to stdout can't fail"); - } - - // see comment above - buf_writer.flush().expect("Flushing stdout can't fail"); - (number_of_warnings, number_of_errors) - } -} - -fn process_paths(paths: &[PathBuf], runtime_data: LinterRuntimeData) -> Result<()> { - paths.par_iter().try_for_each(move |path| { - let path = path.canonicalize().with_path(path)?; - - if path.is_file() { - run_for_file(&path, &runtime_data) - } else if path.is_dir() { - run_for_dir(&path, &runtime_data) - } else { - Ok(()) - } - }) -} - -fn wrap_diagnostics( - path: &Path, - source_text: &str, - diagnostics: Vec, -) -> (PathBuf, Vec) { - let source = Arc::new(NamedSource::new(path.to_string_lossy(), source_text.to_owned())); - - let diagnostics = diagnostics - .into_iter() - .map(|diagnostic| diagnostic.with_source_code(Arc::clone(&source))) - .collect(); - - (path.to_path_buf(), diagnostics) -} - -fn run_for_dir(path: &Path, runtime_data: &LinterRuntimeData) -> Result<()> { - fs::read_dir(path).with_path(path)?.par_bridge().try_for_each(|entry| { - let path = entry.with_path(path)?.path(); - - if path.is_file() { - if !runtime_data.visited.contains(&path) { - run_for_file(&path, runtime_data)?; - } - } else if path.is_dir() { - run_for_dir(&path, runtime_data)?; - } - - Ok(()) - }) -} - -// static RESOLVER: OnceLock = OnceLock::new(); - -fn run_for_file(path: &Path, runtime_data: &LinterRuntimeData) -> Result<()> { - let LinterRuntimeData { linter, visited, tx_error } = &runtime_data; - - if visited.contains(path) { - return Ok(()); - } - - visited.insert(path.to_path_buf()); - - let Ok(source_type) = SourceType::from_path(path) else { - // skip unsupported file types (e.g. .css or .json) - return Ok(()); - }; - - let source = fs::read_to_string(path).with_path(path)?; - - let allocator = Allocator::default(); - - let (program, trivias) = { - let ParserReturn { program, errors, trivias, .. } = - Parser::new(&allocator, &source, source_type).parse(); - - if !errors.is_empty() { - tx_error.send(wrap_diagnostics(path, &source, errors)).unwrap(); - return Ok(()); - }; - - (allocator.alloc(program), trivias) - }; - - let semantic = { - let SemanticBuilderReturn { errors, semantic } = SemanticBuilder::new(&source, source_type) - .with_trivias(&trivias) - .with_check_syntax_error(true) - .with_module_record_builder(true) - .build(program); - - if !errors.is_empty() { - tx_error.send(wrap_diagnostics(path, &source, errors)).unwrap(); - return Ok(()); - }; - - semantic - }; - - // this is ok to unwrap because we know that the resolver is initialized, otherwise this function wouldn't be called - // let resolver = RESOLVER.get().unwrap(); - - // let resolve_path = path.parent().expect("Absolute file path always has a parent"); - - // let imported_modules = semantic.module_record().module_requests.keys(); - - // imported_modules - // .par_bridge() - // .filter(|name| name.starts_with('.')) - // .filter_map(|name| { - // resolver.resolve(resolve_path, name).map_or_else( - // |_| { - // eprintln!("Couldn't resolve '{name}' in '{}'.", resolve_path.display()); - // None - // }, - // Some, - // ) - // }) - // .filter_map(|resolved| match resolved { - // ResolveResult::Resource(r) => Some(r.path), - // ResolveResult::Ignored => None, - // }) - // .filter(|path| { - // path.extension() - // .and_then(OsStr::to_str) - // .is_some_and(|ext| VALID_EXTENSIONS.contains(&ext)) - // }) - // .filter(|path| !visited.contains(path)) - // .try_for_each(|path| run_for_file(&path, runtime_data))?; - - let lint_ctx = LintContext::new(&Rc::new(semantic)); - let result = linter.run(lint_ctx); - - if result.is_empty() { - return Ok(()); - } - - let messages = if linter.has_fix() { - let FixResult { messages, fixed_code, .. } = Fixer::new(&source, result).fix(); - fs::write(path, fixed_code.as_bytes()).with_path(path)?; - messages - } else { - result - }; - - let errors = messages.into_iter().map(|m| m.error).collect(); - let diagnostic = wrap_diagnostics(path, &source, errors); - tx_error.send(diagnostic).unwrap(); - - Ok(()) -} diff --git a/crates/oxc_cli/src/lint/options.rs b/crates/oxc_cli/src/lint/options.rs index 31d10c2e2..5aa89e496 100644 --- a/crates/oxc_cli/src/lint/options.rs +++ b/crates/oxc_cli/src/lint/options.rs @@ -3,9 +3,7 @@ use std::{collections::BTreeMap, env, path::PathBuf}; use clap::ArgMatches; use super::command::lint_command; -pub use super::{ - error::Error, isolated_handler::IsolatedLintHandler, module_tree_handler::ModuleTreeLintHandler, -}; +pub use super::{error::Error, isolated_handler::IsolatedLintHandler}; use crate::runner::RunnerOptions; #[derive(Debug)]