Skip to content

Commit

Permalink
fix: multiple diagnostics
Browse files Browse the repository at this point in the history
  • Loading branch information
h-a-n-a committed Dec 7, 2023
1 parent b50e77a commit c867026
Show file tree
Hide file tree
Showing 11 changed files with 161 additions and 86 deletions.
4 changes: 2 additions & 2 deletions crates/rspack_core/src/compiler/compilation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ pub struct Compilation {
pub async_entrypoints: Vec<ChunkGroupUkey>,
assets: CompilationAssets,
pub emitted_assets: DashSet<String, BuildHasherDefault<FxHasher>>,
diagnostics: IndexSet<Diagnostic, BuildHasherDefault<FxHasher>>,
diagnostics: Vec<Diagnostic>,
logging: CompilationLogging,
pub plugin_driver: SharedPluginDriver,
pub resolver_factory: Arc<ResolverFactory>,
Expand Down Expand Up @@ -333,7 +333,7 @@ impl Compilation {
}

pub fn push_diagnostic(&mut self, diagnostic: Diagnostic) {
self.diagnostics.insert(diagnostic);
self.diagnostics.push(diagnostic);
}

pub fn push_batch_diagnostic(&mut self, diagnostics: Vec<Diagnostic>) {
Expand Down
29 changes: 0 additions & 29 deletions crates/rspack_core/src/compiler/queue.rs
Original file line number Diff line number Diff line change
Expand Up @@ -244,35 +244,6 @@ impl WorkerTask for BuildTask {
let cache = self.cache;
let plugin_driver = self.plugin_driver;

// match cache
// .build_module_occasion
// .use_cache(&mut module, |module| async {
// plugin_driver
// .build_module(module.as_mut())
// .await
// .unwrap_or_else(|e| panic!("Run build_module hook failed: {}", e));

// let result = module
// .build(BuildContext {
// compiler_context: CompilerContext {
// options: compiler_options.clone(),
// resolver_factory: resolver_factory.clone(),
// module: Some(module.identifier()),
// module_context: module.as_normal_module().and_then(|m| m.get_context()),
// },
// plugin_driver: plugin_driver.clone(),
// compiler_options: &compiler_options,
// })
// .await;

// plugin_driver
// .succeed_module(&**module)
// .await
// .unwrap_or_else(|e| panic!("Run succeed_module hook failed: {}", e));

// result.map(|t| (t, module))
// })
// .await
let (build_result, is_cache_valid) = match cache
.build_module_occasion
.use_cache(&mut module, |module| async {
Expand Down
30 changes: 0 additions & 30 deletions crates/rspack_error/src/diagnostic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -62,37 +62,7 @@ impl Deref for Diagnostic {
}
}

/// Shouldn't rely on this in deduping.
/// Have to make sure everything is deduped in the first place.
impl std::hash::Hash for Diagnostic {
fn hash<H: std::hash::Hasher>(&self, state: &mut H) {
self.0.to_string().hash(state);
self.0.code().map(|c| c.to_string()).hash(state);
self.0.help().map(|h| h.to_string()).hash(state);
self.0.url().map(|u| u.to_string()).hash(state);
self.0.severity().map(Severity::from).hash(state);
}
}

/// Shouldn't rely on this in deduping.
/// Have to make sure everything is deduped in the first place.
impl PartialEq for Diagnostic {
fn eq(&self, other: &Self) -> bool {
self.0.to_string() == other.0.to_string()
&& self.0.code().map(|c| c.to_string()) == other.0.code().map(|c| c.to_string())
&& self.0.help().map(|h| h.to_string()) == other.0.help().map(|h| h.to_string())
&& self.0.url().map(|u| u.to_string()) == other.0.url().map(|u| u.to_string())
&& self.0.severity() == other.0.severity()
}
}

impl Eq for Diagnostic {}

impl Diagnostic {
pub fn title(&self) -> String {
self.0.code().map(|v| v.to_string()).unwrap_or_default()
}

pub fn message(&self) -> String {
self.0.to_string()
}
Expand Down
9 changes: 1 addition & 8 deletions crates/rspack_error/src/emitter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,6 @@ use termcolor::{ColorChoice, StandardStream};

use crate::Diagnostic;

/// Detects if's in a terminal.
fn is_terminal() -> bool {
std::io::stdout().is_terminal() && std::io::stderr().is_terminal()
}

pub trait FlushDiagnostic {
fn flush_diagnostic(&mut self) {}
}
Expand Down Expand Up @@ -162,9 +157,7 @@ fn emit_diagnostic<T: Write + WriteColor>(
diagnostic: &Diagnostic,
writer: &mut T,
) -> crate::Result<()> {
let h = GraphicalReportHandler::new().with_theme(if !is_terminal() {
GraphicalTheme::ascii()
} else if writer.supports_color() {
let h = GraphicalReportHandler::new().with_theme(if writer.supports_color() {
GraphicalTheme::unicode()
} else {
GraphicalTheme::unicode_nocolor()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,4 +9,11 @@ expression: html_syntax_error
· ╰── Start tag seen without seeing a doctype first, expected "<!DOCTYPE html>"
2 │ </something>
╰────
× Error[html]: HTML parsing error
╭─[tests/fixtures/html_syntax_error/index.html:1:1]
1 │ <head>
2 │ </something>
· ──────┬─────
· ╰── Stray end tag "something"
╰────

Original file line number Diff line number Diff line change
Expand Up @@ -10,4 +10,11 @@ expression: js_syntax_error
· ╰── Expected a semicolon
3// Comma is required, but parser can recover because of the newline.
╰────
× Error[javascript]: JavaScript parsing error
╭─[tests/fixtures/js_syntax_error/index.js:5:1]
5g = CONST
6 │ }
· ┬
· ╰── Expression expected
╰────

Original file line number Diff line number Diff line change
Expand Up @@ -8,4 +8,8 @@ expression: jsx_syntax_error
· ▲
· ╰── Unexpected token. Did you mean `{'>'}` or `&gt;`?
╰────
× Error[jsx]: JSX parsing error
╭─[tests/fixtures/jsx_syntax_error/index.jsx:1:1]
1 │ let a = <test>/test>;
╰────

Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,12 @@ expression: recoverable_syntax_error
---
× Error[tsx]: TSX parsing error
╭─[tests/fixtures/recoverable_syntax_error/index.tsx:1:1]
1let c = <test>() => {}</test>;
· ┬
· ╰── Expression expected
╰────
× Error[tsx]: TSX parsing error
╭─[tests/fixtures/recoverable_syntax_error/index.tsx:1:1]
1let c = <test>() => {}</test>;
· ┬
· ╰── Expected a semicolon
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,4 +9,8 @@ expression: multi_json_syntax_error
· ┬
· ╰── Unexpected character }
╰────
× Error[json]: Json parsing error
╭─[tests/out_of_order/multi_json_syntax_error/b.json:1:1]
1"测试utf8\"
╰───

Original file line number Diff line number Diff line change
Expand Up @@ -10,4 +10,101 @@ expression: multiple_file_syntax_error
· ╰── Expected a semicolon
3// Comma is required, but parser can recover because of the newline.
╰────
× Error[javascript]: JavaScript parsing error
╭─[tests/out_of_order/multiple_file_syntax_error/a.js:5:1]
5g = CONST
6 │ }
· ┬
· ╰── Expression expected
╰────
× Error[javascript]: JavaScript parsing error
╭─[tests/out_of_order/multiple_file_syntax_error/b.js:1:1]
1const CONST = 9000 % 2;
2const D {
· ┬
· ╰── Expected a semicolon
3// Comma is required, but parser can recover because of the newline.
╰────
× Error[javascript]: JavaScript parsing error
╭─[tests/out_of_order/multiple_file_syntax_error/b.js:5:1]
5g = CONST
6 │ }
· ┬
· ╰── Expression expected
╰────
× Error[javascript]: JavaScript parsing error
╭─[tests/out_of_order/multiple_file_syntax_error/c.js:1:1]
1const CONST = 9000 % 2;
2const D {
· ┬
· ╰── Expected a semicolon
3// Comma is required, but parser can recover because of the newline.
╰────
× Error[javascript]: JavaScript parsing error
╭─[tests/out_of_order/multiple_file_syntax_error/c.js:5:1]
5g = CONST
6 │ }
· ┬
· ╰── Expression expected
╰────
× Error[javascript]: JavaScript parsing error
╭─[tests/out_of_order/multiple_file_syntax_error/d.js:1:1]
1const CONST = 9000 % 2;
2const D {
· ┬
· ╰── Expected a semicolon
3// Comma is required, but parser can recover because of the newline.
╰────
× Error[javascript]: JavaScript parsing error
╭─[tests/out_of_order/multiple_file_syntax_error/d.js:5:1]
5g = CONST
6 │ }
· ┬
· ╰── Expression expected
╰────
× Error[javascript]: JavaScript parsing error
╭─[tests/out_of_order/multiple_file_syntax_error/e.js:1:1]
1const CONST = 9000 % 2;
2const D {
· ┬
· ╰── Expected a semicolon
3// Comma is required, but parser can recover because of the newline.
╰────
× Error[javascript]: JavaScript parsing error
╭─[tests/out_of_order/multiple_file_syntax_error/e.js:5:1]
5g = CONST
6 │ }
· ┬
· ╰── Expression expected
╰────
× Error[javascript]: JavaScript parsing error
╭─[tests/out_of_order/multiple_file_syntax_error/f.js:1:1]
1const CONST = 9000 % 2;
2const D {
· ┬
· ╰── Expected a semicolon
3// Comma is required, but parser can recover because of the newline.
╰────
× Error[javascript]: JavaScript parsing error
╭─[tests/out_of_order/multiple_file_syntax_error/f.js:5:1]
5g = CONST
6 │ }
· ┬
· ╰── Expression expected
╰────
× Error[javascript]: JavaScript parsing error
╭─[tests/out_of_order/multiple_file_syntax_error/g.js:1:1]
1const CONST = 9000 % 2;
2const D {
· ┬
· ╰── Expected a semicolon
3// Comma is required, but parser can recover because of the newline.
╰────
× Error[javascript]: JavaScript parsing error
╭─[tests/out_of_order/multiple_file_syntax_error/g.js:5:1]
5g = CONST
6 │ }
· ┬
· ╰── Expression expected
╰────

50 changes: 33 additions & 17 deletions crates/rspack_plugin_copy/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use std::{
fs,
hash::Hash,
path::{Path, PathBuf, MAIN_SEPARATOR},
sync::Arc,
sync::{Arc, Mutex},
};

use async_trait::async_trait;
Expand Down Expand Up @@ -137,7 +137,7 @@ impl CopyRspackPlugin {
output_path: &Path,
from_type: FromType,
file_dependencies: &DashSet<PathBuf>,
diagnostics: &DashSet<Diagnostic>,
diagnostics: &Mutex<Vec<Diagnostic>>,
compilation: &Compilation,
logger: &CompilationLogger,
) -> Option<RunPatternResult> {
Expand Down Expand Up @@ -234,9 +234,10 @@ impl CopyRspackPlugin {
Err(e) => {
let e: Error = DiagnosticError::from(e.boxed()).into();
let rspack_err: Vec<Diagnostic> = vec![e.into()];
for err in rspack_err {
diagnostics.insert(err);
}
diagnostics
.lock()
.expect("failed to obtain lock of `diagnostics`")
.extend(rspack_err);
return None;
}
};
Expand Down Expand Up @@ -290,7 +291,7 @@ impl CopyRspackPlugin {
_index: usize,
file_dependencies: &DashSet<PathBuf>,
context_dependencies: &DashSet<PathBuf>,
diagnostics: &DashSet<Diagnostic>,
diagnostics: &Mutex<Vec<Diagnostic>>,
logger: &CompilationLogger,
) -> Option<Vec<Option<RunPatternResult>>> {
let orig_from = &pattern.from;
Expand Down Expand Up @@ -431,10 +432,13 @@ impl CopyRspackPlugin {
return None;
}

diagnostics.insert(Diagnostic::error(
"CopyRspackPlugin Error".into(),
format!("unable to locate '{glob_query}' glob"),
));
diagnostics
.lock()
.expect("failed to obtain lock of `diagnostics`")
.push(Diagnostic::error(
"CopyRspackPlugin Error".into(),
format!("unable to locate '{glob_query}' glob"),
));
}

let output_path = &compilation.options.output.path;
Expand Down Expand Up @@ -463,10 +467,13 @@ impl CopyRspackPlugin {
}

// TODO err handler
diagnostics.insert(Diagnostic::error(
"CopyRspackPlugin Error".into(),
format!("unable to locate '{glob_query}' glob"),
));
diagnostics
.lock()
.expect("failed to obtain lock of `diagnostics`")
.push(Diagnostic::error(
"CopyRspackPlugin Error".into(),
format!("unable to locate '{glob_query}' glob"),
));
return None;
}

Expand All @@ -484,7 +491,10 @@ impl CopyRspackPlugin {
return None;
}

diagnostics.insert(Diagnostic::error("Glob Error".into(), e.msg.to_string()));
diagnostics
.lock()
.expect("failed to obtain lock of `diagnostics`")
.push(Diagnostic::error("Glob Error".into(), e.msg.to_string()));

None
}
Expand All @@ -507,7 +517,7 @@ impl Plugin for CopyRspackPlugin {
let start = logger.time("run pattern");
let file_dependencies = DashSet::default();
let context_dependencies = DashSet::default();
let diagnostics = DashSet::default();
let diagnostics = Mutex::new(Vec::new());

let mut copied_result: Vec<(i32, RunPatternResult)> = self
.patterns
Expand Down Expand Up @@ -552,7 +562,13 @@ impl Plugin for CopyRspackPlugin {
compilation
.context_dependencies
.extend(context_dependencies);
compilation.push_batch_diagnostic(diagnostics.into_iter().collect());
compilation.push_batch_diagnostic(
diagnostics
.lock()
.expect("failed to obtain lock of `diagnostics`")
.drain(..)
.collect(),
);

copied_result.sort_unstable_by(|a, b| a.0.cmp(&b.0));
copied_result.into_iter().for_each(|(_priority, result)| {
Expand Down

0 comments on commit c867026

Please sign in to comment.