-
Notifications
You must be signed in to change notification settings - Fork 484
Restore legacy (. ...) uncurried syntax with deprecation warning
#8383
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 2 commits
6cdc76d
adb9891
129bf58
df04831
8267807
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -940,10 +940,13 @@ fn compile_file( | |
|
|
||
| if helpers::contains_ascii_characters(&err) { | ||
| if package.is_local_dep { | ||
| // suppress warnings of external deps | ||
| Ok(Some(err)) | ||
| } else { | ||
| Ok(None) | ||
| // Warnings from external deps are suppressed by default — | ||
| // users can't act on them. A small allow-list of critical | ||
| // deprecations still gets through so breakage signals are | ||
| // visible (and can be reported upstream). | ||
| Ok(retain_critical_external_warnings(&err)) | ||
| } | ||
| } else { | ||
| Ok(None) | ||
|
|
@@ -952,6 +955,33 @@ fn compile_file( | |
| } | ||
| } | ||
|
|
||
| /// Filter a bsc stderr capture to the warning blocks the user needs to see | ||
| /// even when they originate in an external dependency. | ||
| /// | ||
| /// Currently preserved: | ||
| /// - Warning 3 deprecations mentioning the legacy `(. ...)` uncurried syntax. | ||
| /// These indicate source that parses today but is scheduled for removal, so | ||
| /// consumers need to hear about them even when the code isn't theirs. | ||
| pub(super) fn retain_critical_external_warnings(stderr: &str) -> Option<String> { | ||
| const UNCURRIED_DOT_MARKER: &str = "`(. ...)` uncurried syntax"; | ||
| if !stderr.contains(UNCURRIED_DOT_MARKER) { | ||
| return None; | ||
| } | ||
| // bsc prints each warning as its own block separated by a blank-line pair | ||
| // (three consecutive newlines). Split on that boundary, keep the blocks | ||
| // that mention the marker, and re-join with the same separator so the | ||
| // output is indistinguishable from the original. | ||
| let kept: Vec<&str> = stderr | ||
| .split("\n\n\n") | ||
| .filter(|block| block.contains(UNCURRIED_DOT_MARKER)) | ||
|
Comment on lines
+1132
to
+1133
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
The filter assumes warning blocks are separated by Useful? React with 👍 / 👎. |
||
| .collect(); | ||
| if kept.is_empty() { | ||
| None | ||
| } else { | ||
| Some(kept.join("\n\n\n")) | ||
| } | ||
| } | ||
|
|
||
| pub fn mark_modules_with_deleted_deps_dirty(build_state: &mut BuildState) { | ||
| build_state.modules.iter_mut().for_each(|(_, module)| { | ||
| if !module.deps.is_disjoint(&build_state.deleted_modules) { | ||
|
|
@@ -1048,3 +1078,26 @@ pub fn mark_modules_with_expired_deps_dirty(build_state: &mut BuildCommandState) | |
| } | ||
| }); | ||
| } | ||
|
|
||
| #[cfg(test)] | ||
| mod tests { | ||
| use super::*; | ||
|
|
||
| #[test] | ||
| fn retain_critical_external_warnings_returns_none_without_marker() { | ||
| let input = "\n Warning number 26\n foo.res:1:1\n\n unused variable x.\n"; | ||
| assert_eq!(retain_critical_external_warnings(input), None); | ||
| } | ||
|
|
||
| #[test] | ||
| fn retain_critical_external_warnings_keeps_uncurried_dot_block() { | ||
| let input = concat!( | ||
| "\n Warning number 26\n foo.res:1:1\n\n unused variable x.\n", | ||
| "\n\n\n Warning number 3\n bar.res:5:10\n\n ", | ||
| "deprecated: The `(. ...)` uncurried syntax is deprecated.\n", | ||
| ); | ||
| let kept = retain_critical_external_warnings(input).expect("uncurried-dot warning should survive"); | ||
| assert!(kept.contains("`(. ...)` uncurried syntax")); | ||
| assert!(!kept.contains("unused variable")); | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,82 @@ | ||
| #!/bin/bash | ||
| # Verifies that the legacy `(. args)` uncurried-syntax deprecation warning | ||
| # is surfaced even when it originates from an external (node_modules) | ||
| # dependency. Unrelated warnings from external deps remain suppressed. | ||
|
|
||
| cd $(dirname $0) | ||
| source "../utils.sh" | ||
|
|
||
| bold "Test: Uncurried-dot deprecation warning surfaces from external deps" | ||
|
|
||
| fixture=$(mktemp -d 2>/dev/null || mktemp -d -t rewatch-ext-uncur) | ||
| trap "rm -rf '$fixture'" EXIT | ||
|
|
||
| mkdir -p "$fixture/src" | ||
| mkdir -p "$fixture/node_modules/legacy-pkg/src" | ||
|
|
||
| cat > "$fixture/package.json" <<'EOF' | ||
| { | ||
| "name": "host", | ||
| "version": "0.0.1" | ||
| } | ||
| EOF | ||
|
|
||
| cat > "$fixture/rescript.json" <<'EOF' | ||
| { | ||
| "name": "host", | ||
| "sources": { "dir": "src" }, | ||
| "dependencies": ["legacy-pkg"], | ||
| "package-specs": { "module": "commonjs", "in-source": true }, | ||
| "suffix": ".bs.js" | ||
| } | ||
| EOF | ||
|
|
||
| cat > "$fixture/src/Main.res" <<'EOF' | ||
| let _ = LegacyPkg.add(1, 2) | ||
| EOF | ||
|
|
||
| cat > "$fixture/node_modules/legacy-pkg/package.json" <<'EOF' | ||
| { "name": "legacy-pkg", "version": "0.0.1" } | ||
| EOF | ||
|
|
||
| cat > "$fixture/node_modules/legacy-pkg/rescript.json" <<'EOF' | ||
| { | ||
| "name": "legacy-pkg", | ||
| "sources": { "dir": "src" }, | ||
| "package-specs": { "module": "commonjs", "in-source": true }, | ||
| "suffix": ".bs.js" | ||
| } | ||
| EOF | ||
|
|
||
| # Includes: the uncurried-dot deprecation (must surface) and an unused value | ||
| # (warning 26, must stay suppressed because it's in an external dep). | ||
| cat > "$fixture/node_modules/legacy-pkg/src/LegacyPkg.res" <<'EOF' | ||
| let unusedInDep = 99 | ||
| let add = (. a, b) => a + b | ||
| EOF | ||
|
|
||
| cd "$fixture" | ||
| stderr_output=$(rewatch 2>&1 1>/dev/null) | ||
| build_status=$? | ||
|
|
||
| if [ $build_status -ne 0 ]; then | ||
| error "Build failed" | ||
| printf "%s\n" "$stderr_output" >&2 | ||
| exit 1 | ||
| fi | ||
|
|
||
| if echo "$stderr_output" | grep -qF '`(. ...)` uncurried syntax'; then | ||
| success "Uncurried-dot deprecation warning is shown for external dep" | ||
| else | ||
| error "Expected '(. ...)' uncurried-syntax deprecation in stderr" | ||
| printf "%s\n" "$stderr_output" >&2 | ||
| exit 1 | ||
| fi | ||
|
|
||
| if echo "$stderr_output" | grep -qF 'unused value'; then | ||
| error "Unrelated warnings from external deps should be suppressed" | ||
| printf "%s\n" "$stderr_output" >&2 | ||
| exit 1 | ||
| else | ||
| success "Unrelated external-dep warnings are still suppressed" | ||
| fi |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.