Skip to content

Conversation

@gwennlbh
Copy link
Contributor

@gwennlbh gwennlbh commented Sep 6, 2025

Fixes #13570

@graphite-app
Copy link
Contributor

graphite-app bot commented Sep 6, 2025

How to use the Graphite Merge Queue

Add either label to this PR to merge it via the merge queue:

  • 0-merge - adds this PR to the back of the merge queue
  • hotfix - for urgent hot fixes, skip the queue and merge this PR next

You must have a Graphite account in order to use the merge queue. Sign up using this link.

An organization admin has enabled the Graphite Merge Queue in this repository.

Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue.

@github-actions github-actions bot added A-linter Area - Linter A-semantic Area - Semantic C-bug Category - Bug labels Sep 6, 2025
@gwennlbh gwennlbh changed the title fix(semantic): dont parse @as jsodc tags inside quotes fix(semantic): dont parse @ as jsodc tags inside quotes Sep 6, 2025
@gwennlbh gwennlbh changed the title fix(semantic): dont parse @ as jsodc tags inside quotes fix(semantic): dont parse @ as jsdoc tags inside quotes Sep 6, 2025
@gwennlbh gwennlbh force-pushed the main branch 3 times, most recently from 8013334 to d24d3d5 Compare September 6, 2025 22:34
@gwennlbh
Copy link
Contributor Author

gwennlbh commented Sep 6, 2025

just ready passes locally, except for

  • sourcemap::stacktrace_is_correct, there's my local full path from the C: drive in the .new snapshot
  • lint::test::ignore_patterns_with_symlink, false negative, says that "debugger is not allowed' when it should return no errors

I highly suspect that those two fail because of Windows shenanigans, here's a patch for the Justfile in case it's useful

Justfile
commit 05673760f9847ccae0375e4da4e26acd06d3fdd5
Author: Gwenn Le Bihan <gwenn.lebihan7@gmail.com>
Date:   Sun Sep 7 00:43:36 2025 +0200

    chore(just): disable windows-borked tests on windows

diff --git a/justfile b/justfile
index 5c6f00131..6f5a604e6 100755
--- a/justfile
+++ b/justfile
@@ -74,9 +74,14 @@ check:
   cargo ck
 
 # Run all the tests
+[unix]
 test:
   cargo test --all-features
 
+[windows]
+test:
+  cargo test --all-features -- --skip sourcemap::stacktrace_is_correct --skip lint::test::ignore_patterns_with_symlink
+
 # Lint the whole project
 lint:
   cargo lint -- --deny warnings

There's also a small failure while checking docs, because there's a URL without angle brackets in some doc-comment, again, another patch:

crates/oxc_diagnostics/src/service.rs
commit f18813815ec075b18e21f3a3c295b40684e6097a
Author: Gwenn Le Bihan <gwenn.lebihan7@gmail.com>
Date:   Sun Sep 7 00:58:15 2025 +0200

    chore(doc): fix missing <> around hyperlink

diff --git a/crates/oxc_diagnostics/src/service.rs b/crates/oxc_diagnostics/src/service.rs
index e2422de58..bf9fe0595 100644
--- a/crates/oxc_diagnostics/src/service.rs
+++ b/crates/oxc_diagnostics/src/service.rs
@@ -296,7 +296,7 @@ fn from_file_path<A: AsRef<Path>>(path: A) -> Option<String> {
 }
 
 /// On Windows, rewrites the wide path prefix `\\?\C:` to `C:`
-/// Source: https://stackoverflow.com/a/70970317
+/// Source: <https://stackoverflow.com/a/70970317>
 #[inline]
 #[cfg(windows)]
 fn strict_canonicalize<P: AsRef<Path>>(path: P) -> std::io::Result<PathBuf> {

I also smoketested my new test (it fails without my change to oxc_semantics)

@gwennlbh gwennlbh marked this pull request as ready for review September 6, 2025 22:36
Copilot AI review requested due to automatic review settings September 6, 2025 22:36
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR fixes an issue where the JSDoc parser incorrectly identified @ characters inside quoted strings as JSDoc tags. The fix ensures that @ characters within single or double quotes are ignored when parsing JSDoc comments.

  • Adds quote tracking to the JSDoc parser to prevent parsing @ symbols inside quoted strings
  • Updates the parsing condition to exclude quote contexts from tag detection
  • Adds test coverage for the specific case of @import statements with quoted package names

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
crates/oxc_semantic/src/jsdoc/parser/parse.rs Implements quote tracking logic to prevent @ parsing inside quoted strings
crates/oxc_linter/src/rules/jsdoc/check_tag_names.rs Adds test case for @import with quoted package name to verify the fix

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@gwennlbh
Copy link
Contributor Author

gwennlbh commented Sep 6, 2025

just in case, the three patches I commented are available on https://github.com/gwennlbh/oxc/tree/otherfixes

@codspeed-hq
Copy link

codspeed-hq bot commented Sep 7, 2025

CodSpeed Instrumentation Performance Report

Merging #13571 will not alter performance

Comparing gwennlbh:main (17ac6c4) with main (5648b31)

Summary

✅ 37 untouched benchmarks

@camc314 camc314 self-assigned this Sep 7, 2025
Copy link
Contributor

@camc314 camc314 left a comment

Choose a reason for hiding this comment

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

Copy link
Contributor

@camc314 camc314 left a comment

Choose a reason for hiding this comment

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

thank you!

@camc314 camc314 merged commit 198243b into oxc-project:main Sep 8, 2025
25 checks passed
camc314 added a commit that referenced this pull request Sep 11, 2025
## [1.15.0] - 2025-09-11

### 💥 BREAKING CHANGES

- edc70ea allocator/pool: [**BREAKING**] Remove `disable_fixed_size`
Cargo feature (#13625) (overlookmotel)

### 🚀 Features

- b20b56d linter: Add `vue/no-multiple-slot-args` rule (#13579) (Sysix)
- aafe08c linter: Add `vue/define-emits-declaration` rule (#13567)
(Sysix)
- 2ed5059 linter: Add `vue/define-props-declaration` rule (#13566)
(Sysix)
- a718c23 linter: Add `vue/valid-define-props` rule (#13565) (Sysix)
- 75a673e editor: Support relative path for `oxc.path.server` (#13542)
(Sysix)
- 4af886b linter: Add `unicorn/no-array-reverse` rule (#13530) (yefan)
- 2db32eb data_structures: Add `boxed_slice!` and `boxed_array!` macros
(#13596) (overlookmotel)

### 🐛 Bug Fixes

- fb9d0f4 language_server: Don't resend diagnostic on save, when
`typeAware` is disabled and run is onType (#13604) (YongSeok Jang (장용석))
- 2f36350 editor: Add notice for a possible restart when fixing
`filename-case` (#13557) (Sysix)
- e17fccc linter: Update `RuleRunner` impl after merge (#13642)
(camc314)
- 3d27c5b linter/no-unused-private-class-members: False positive with
spread expr (#13634) (yefan)
- 8314ed5 linter/tsgolint: Correct comment (#13589) (camc314)
- 198243b semantic: Dont parse `@` as jsdoc tags inside quotes (#13571)
(Gwenn Le Bihan)
- 89084d7 linter/custom-plugins: Enforce exact matching for disable
directives (#13538) (Copilot)
- 277c5e1 linter: Output `eslint-plugin-vue` for vue diagnostics
(#13564) (Sysix)
- 34d3cde rust: Fix clippy issues (#13540) (Boshen)
- 5fccafc linter: `unicorn/prefer-array-flat-map` ignore
`React.Children` (#13534) (Sysix)
- 7e78e39 linter: Don't panic when parsing regex with multiple
parentheses (#13524) (Sysix)
- 0d867b1 linter: Skip running tsgolint when no files need type aware
linting (#13502) (Copilot)
- b677376 language_server: Include the diagnostic of the other linter
(#13490) (Sysix)
- e87d7bd linter: Parse regex inside `new RegExp()` with parentheses
(#13448) (Sysix)
- 5990f17 linter: Change `typescript/no-confusing-void-expression` to
pedantic (#13473) (Boshen)

### 🚜 Refactor

- 7775c21 linter/plugins: Remove `oxlint2` Cargo feature (#13648)
(overlookmotel)
- 8f37e88 linter: Update tsgolint payload (#13547) (camchenry)
- 2d53203 linter/plugins: Move `tokio` usage from `oxc_linter` to
`napi/oxlint2` (#13647) (overlookmotel)
- 6cd6be2 linter: Add `--experimental-js-plugins` CLI arg (#13658)
(overlookmotel)
- 476729b linter: Simplify `RuleRunner` trait definition (#13637)
(camchenry)
- 2f02ac6 linter/plugins: Remove `disable_oxlint2` Cargo feature
(#13626) (overlookmotel)
- ff9e4fb linter/plugins: Use fixed-size allocators when
`ExternalLinter` exists (#13623) (overlookmotel)
- f9bff64 linter_codegen: Improve code style for collecting nodes
(#13636) (camchenry)
- babbaca all: Remove `pub` from modules with no exports (#13618)
(overlookmotel)
- 91759c6 linter/plugins: Only use `RawTransferFileSystem` if JS plugins
registered (#13599) (overlookmotel)
- 118020c linter/plugins: Discard `ExternalLinter` if no JS plugins
registered (#13598) (overlookmotel)
- 8d30bce linter/tsgolint: Report an error if the tsgolint exe could not
be found (#13590) (camc314)
- bccc276 eslint/for-direction: Clean up implementation and improve
documentation (#13532) (Antoine Zanardi)
- 1425da2 eslint/default-case-last: Simplify default case last check in
switch statement (#13529) (Antoine Zanardi)
- d245376 oxlint: Remove unused `runner` module (#13561) (camc314)
- 53f2fc1 eslint/default-case: Simplify implementation and enhance
readability (#13430) (Antoine Zanardi)
- 6f15060 eslint/block-scoped-var: Clean up implementation and improve
documentation (#13417) (Antoine Zanardi)
- 671e0fd language_server: Only store one instance of a diagnostic
(#13514) (Sysix)
- 1b425d6 eslint/default-case-last: Simplify implementation and enhance
readability (#13515) (Antoine Zanardi)
- e4bbbce eslint/default-param-last: Simplify implementation and enhance
readability (#13516) (Antoine Zanardi)
- e0396fd linter: Remove `static` lifetime from disable directives
function argument (#13492) (camc314)

### 📚 Documentation

- eb1f167 linter: Note which rules require type info to run on rule page
(#13675) (camc314)
- e66f93b linter: Fix backtick formatting in no-return-wrap (#13633)
(camc314)

### ⚡ Performance

- e6a25e7 linter: Remove unnecessary `should_run` check (#13639)
(camchenry)
- f6a9687 linter: Store rules by AST type in a boxed array (#13578)
(overlookmotel)
- b81f081 linter: Reduce indirection (#13574) (overlookmotel)
- a744aff linter: Skip rules that do not have any relevant node types
(#13138) (camchenry)

### 🎨 Styling

- e110476 linter: Reformat code (#13573) (overlookmotel)

### 🧪 Testing

- 58e6c94 oxlint: Add test for ignorePatterns whitelist (#13372) (Sysix)

Co-authored-by: camc314 <18101008+camc314@users.noreply.github.com>
gwennlbh added a commit to cigaleapp/cigale that referenced this pull request Sep 16, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-linter Area - Linter A-semantic Area - Semantic C-bug Category - Bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

linter: jsdoc/check-tag-names falsely flags scoped npm modules when using @import

2 participants