Skip to content

Commit

Permalink
fix: detect magic comments around expr (#7047)
Browse files Browse the repository at this point in the history
  • Loading branch information
@ahabhgk
ahabhgk authored Jul 5, 2024
1 parent d9d3ef0 commit 8d28901
Show file tree
Hide file tree
Showing 5 changed files with 146 additions and 125 deletions.
267 changes: 146 additions & 121 deletions crates/rspack_plugin_javascript/src/webpack_comment.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ use once_cell::sync::Lazy;
use regex::Captures;
use rspack_error::miette::{Diagnostic, Severity};
use rustc_hash::{FxHashMap, FxHashSet};
use swc_core::common::comments::{CommentKind, Comments};
use swc_core::common::comments::{Comment, CommentKind, Comments};
use swc_core::common::{SourceFile, Span};

use crate::visitors::create_traceable_error;
Expand Down Expand Up @@ -100,146 +100,171 @@ static WEBPACK_MAGIC_COMMENT_REGEXP: Lazy<regex::Regex> = Lazy::new(|| {
pub fn try_extract_webpack_magic_comment(
source_file: &SourceFile,
comments: &Option<&dyn Comments>,
import_span: Span,
error_span: Span,
span: Span,
warning_diagnostics: &mut Vec<Box<dyn Diagnostic + Send + Sync>>,
) -> WebpackCommentMap {
let mut result = WebpackCommentMap::new();
comments.with_leading(span.lo, |comments| {
// TODO: remove this, parser.comments contains two same block comment
let mut parsed_comment = FxHashSet::<Span>::default();
for comment in comments
.iter()
.rev()
.filter(|c| matches!(c.kind, CommentKind::Block))
{
if parsed_comment.contains(&comment.span) {
continue;
}
parsed_comment.insert(comment.span);
for captures in WEBPACK_MAGIC_COMMENT_REGEXP.captures_iter(&comment.text) {
if let Some(item_name_match) = captures.name("_0") {
let item_name = item_name_match.as_str();
match item_name {
"webpackChunkName" => {
if let Some(item_value_match) = captures
.name("_1")
.or(captures.name("_2"))
.or(captures.name("_3"))
{
result.insert(
WebpackComment::ChunkName,
item_value_match.as_str().to_string(),
);
} else {
add_magic_comment_warning(
source_file,
item_name,
"a string",
&captures,
warning_diagnostics,
import_span,
);
}
analyze_comments(
source_file,
comments,
error_span,
warning_diagnostics,
&mut result,
)
});
comments.with_trailing(span.hi, |comments| {
analyze_comments(
source_file,
comments,
error_span,
warning_diagnostics,
&mut result,
)
});
result
}

fn analyze_comments(
source_file: &SourceFile,
comments: &[Comment],
error_span: Span,
warning_diagnostics: &mut Vec<Box<dyn Diagnostic + Send + Sync>>,
result: &mut WebpackCommentMap,
) {
// TODO: remove this, parser.comments contains two same block comment
let mut parsed_comment = FxHashSet::<Span>::default();
for comment in comments
.iter()
.rev()
.filter(|c| matches!(c.kind, CommentKind::Block))
{
if parsed_comment.contains(&comment.span) {
continue;
}
parsed_comment.insert(comment.span);
for captures in WEBPACK_MAGIC_COMMENT_REGEXP.captures_iter(&comment.text) {
if let Some(item_name_match) = captures.name("_0") {
let item_name = item_name_match.as_str();
match item_name {
"webpackChunkName" => {
if let Some(item_value_match) = captures
.name("_1")
.or(captures.name("_2"))
.or(captures.name("_3"))
{
result.insert(
WebpackComment::ChunkName,
item_value_match.as_str().to_string(),
);
} else {
add_magic_comment_warning(
source_file,
item_name,
"a string",
&captures,
warning_diagnostics,
error_span,
);
}
"webpackPrefetch" => {
if let Some(item_value_match) = captures.name("_4").or(captures.name("_5")) {
result.insert(
WebpackComment::Prefetch,
item_value_match.as_str().to_string(),
);
} else {
add_magic_comment_warning(
source_file,
item_name,
"true or a number",
&captures,
warning_diagnostics,
import_span,
);
}
}
"webpackPrefetch" => {
if let Some(item_value_match) = captures.name("_4").or(captures.name("_5")) {
result.insert(
WebpackComment::Prefetch,
item_value_match.as_str().to_string(),
);
} else {
add_magic_comment_warning(
source_file,
item_name,
"true or a number",
&captures,
warning_diagnostics,
error_span,
);
}
"webpackPreload" => {
if let Some(item_value_match) = captures.name("_4").or(captures.name("_5")) {
result.insert(
WebpackComment::Preload,
item_value_match.as_str().to_string(),
);
} else {
add_magic_comment_warning(
source_file,
item_name,
"true or a number",
&captures,
warning_diagnostics,
import_span,
);
}
}
"webpackPreload" => {
if let Some(item_value_match) = captures.name("_4").or(captures.name("_5")) {
result.insert(
WebpackComment::Preload,
item_value_match.as_str().to_string(),
);
} else {
add_magic_comment_warning(
source_file,
item_name,
"true or a number",
&captures,
warning_diagnostics,
error_span,
);
}
"webpackIgnore" => {
if let Some(item_value_match) = captures.name("_5") {
result.insert(
WebpackComment::Ignore,
item_value_match.as_str().to_string(),
);
} else {
add_magic_comment_warning(
source_file,
item_name,
"true or false",
&captures,
warning_diagnostics,
import_span,
);
}
}
"webpackIgnore" => {
if let Some(item_value_match) = captures.name("_5") {
result.insert(
WebpackComment::Ignore,
item_value_match.as_str().to_string(),
);
} else {
add_magic_comment_warning(
source_file,
item_name,
"true or false",
&captures,
warning_diagnostics,
error_span,
);
}
}
"webpackMode" => {
if let Some(item_value_match) = captures
.name("_1")
.or(captures.name("_2"))
.or(captures.name("_3"))
{
result.insert(WebpackComment::Mode, item_value_match.as_str().to_string());
} else {
add_magic_comment_warning(
source_file,
item_name,
"a string",
&captures,
warning_diagnostics,
error_span,
);
}
"webpackMode" => {
if let Some(item_value_match) = captures
.name("_1")
.or(captures.name("_2"))
.or(captures.name("_3"))
{
result.insert(WebpackComment::Mode, item_value_match.as_str().to_string());
}
"webpackFetchPriority" => {
if let Some(item_value_match) = captures
.name("_1")
.or(captures.name("_2"))
.or(captures.name("_3"))
{
let priority = item_value_match.as_str();
if priority == "low" || priority == "high" || priority == "auto" {
result.insert(WebpackComment::FetchPriority, priority.to_string());
return;
} else {
add_magic_comment_warning(
source_file,
item_name,
"a string",
r#""low", "high" or "auto""#,
&captures,
warning_diagnostics,
import_span,
error_span,
);
}
}
"webpackFetchPriority" => {
if let Some(item_value_match) = captures
.name("_1")
.or(captures.name("_2"))
.or(captures.name("_3"))
{
let priority = item_value_match.as_str();
if priority == "low" || priority == "high" || priority == "auto" {
result.insert(WebpackComment::FetchPriority, priority.to_string());
return;
} else {
add_magic_comment_warning(
source_file,
item_name,
r#""low", "high" or "auto""#,
&captures,
warning_diagnostics,
import_span,
);
}
}
}
_ => {
// TODO: other magic comment
}
}
_ => {
// TODO: other magic comment
}
}
}
}
});
result
}
}
1 change: 0 additions & 1 deletion tests/webpack-test/configCases/trusted-types/custom-policy-name/test.filter.js
View file

This file was deleted.

1 change: 0 additions & 1 deletion tests/webpack-test/configCases/trusted-types/default-policy-name/test.filter.js
View file

This file was deleted.

1 change: 0 additions & 1 deletion tests/webpack-test/configCases/trusted-types/no-policy-name/test.filter.js
View file

This file was deleted.

1 change: 0 additions & 1 deletion tests/webpack-test/configCases/trusted-types/no-trusted-types/test.filter.js
View file

This file was deleted.

2 comments on commit 8d28901

@rspack-bot
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📝 Ran ecosystem CI: Open

suite result
modernjs ❌ failure
_selftest ✅ success
nx ✅ success
rspress ✅ success
rsbuild ✅ success
compat ✅ success
examples ✅ success

@rspack-bot
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📝 Benchmark detail: Open

Name Base (2024-07-05 bb9841a) Current Change
10000_development-mode + exec 2.13 s ± 18 ms 2.16 s ± 18 ms +1.32 %
10000_development-mode_hmr + exec 694 ms ± 6.4 ms 695 ms ± 2.2 ms +0.07 %
10000_production-mode + exec 2.82 s ± 22 ms 2.83 s ± 44 ms +0.19 %
arco-pro_development-mode + exec 1.88 s ± 77 ms 1.9 s ± 47 ms +0.78 %
arco-pro_development-mode_hmr + exec 434 ms ± 2.1 ms 434 ms ± 1.9 ms -0.11 %
arco-pro_production-mode + exec 3.44 s ± 115 ms 3.44 s ± 86 ms -0.02 %
threejs_development-mode_10x + exec 1.58 s ± 21 ms 1.57 s ± 19 ms -0.52 %
threejs_development-mode_10x_hmr + exec 797 ms ± 12 ms 794 ms ± 4.3 ms -0.36 %
threejs_production-mode_10x + exec 5.59 s ± 31 ms 5.63 s ± 37 ms +0.74 %

Please sign in to comment.