From b5aea7f970ce3adf38d731efb9ebb8a699b51aab Mon Sep 17 00:00:00 2001 From: Yuji Sugiura Date: Fri, 8 Mar 2024 16:42:07 +0900 Subject: [PATCH 01/10] Add @access related tags --- crates/oxc_semantic/src/jsdoc/mod.rs | 1 + crates/oxc_semantic/src/jsdoc/parser/jsdoc.rs | 3 +++ .../src/jsdoc/parser/jsdoc_tag.rs | 21 ++++++++++++------- crates/oxc_semantic/src/jsdoc/parser/mod.rs | 1 + crates/oxc_semantic/src/jsdoc/parser/parse.rs | 5 +++++ crates/oxc_semantic/src/lib.rs | 2 +- 6 files changed, 24 insertions(+), 9 deletions(-) diff --git a/crates/oxc_semantic/src/jsdoc/mod.rs b/crates/oxc_semantic/src/jsdoc/mod.rs index 0caa878fbd7a1..6a6dde05b9e6f 100644 --- a/crates/oxc_semantic/src/jsdoc/mod.rs +++ b/crates/oxc_semantic/src/jsdoc/mod.rs @@ -4,3 +4,4 @@ mod parser; pub use builder::JSDocBuilder; pub use finder::JSDocFinder; +pub use parser::JSDocTag; diff --git a/crates/oxc_semantic/src/jsdoc/parser/jsdoc.rs b/crates/oxc_semantic/src/jsdoc/parser/jsdoc.rs index 91c59de22c133..ccc08a6042f0e 100644 --- a/crates/oxc_semantic/src/jsdoc/parser/jsdoc.rs +++ b/crates/oxc_semantic/src/jsdoc/parser/jsdoc.rs @@ -24,4 +24,7 @@ impl<'a> JSDoc<'a> { let cache = self.cached.get_or_init(|| JSDocParser::new(self.raw).parse()); &cache.1 } + + // Should implement here...? Or leave it to usecases? + // fn has_deprecated_tag(&self) -> bool {} } diff --git a/crates/oxc_semantic/src/jsdoc/parser/jsdoc_tag.rs b/crates/oxc_semantic/src/jsdoc/parser/jsdoc_tag.rs index e815531088a90..edf9ceb0ef8c3 100644 --- a/crates/oxc_semantic/src/jsdoc/parser/jsdoc_tag.rs +++ b/crates/oxc_semantic/src/jsdoc/parser/jsdoc_tag.rs @@ -49,12 +49,16 @@ pub struct Param<'a> { // Structs // -// See https://github.com/microsoft/TypeScript/blob/2d70b57df4b64a3daef252abb014562e6ccc8f3c/src/compiler/types.ts#L397 #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub enum JSDocTagKind<'a> { - Deprecated, // JSDocDeprecatedTag - Parameter(Param<'a>), // JSDocParameterTag - Unknown(&'a str), // JSDocTag + Access, + Deprecated, + Package, + Parameter(Param<'a>), + Private, + Protected, + Public, + Unknown(&'a str), } #[derive(Debug, Clone, PartialEq, Eq)] @@ -66,15 +70,16 @@ pub struct JSDocTag<'a> { impl<'a> JSDocTag<'a> { pub fn tag_name(&self) -> &'a str { match self.kind { + JSDocTagKind::Access => "access", JSDocTagKind::Deprecated => "deprecated", + JSDocTagKind::Package => "package", JSDocTagKind::Parameter(_) => "param", + JSDocTagKind::Private => "private", + JSDocTagKind::Protected => "protected", + JSDocTagKind::Public => "public", JSDocTagKind::Unknown(tag_name) => tag_name, } } - - pub fn is_deprecated(&self) -> bool { - self.kind == JSDocTagKind::Deprecated - } } #[cfg(test)] diff --git a/crates/oxc_semantic/src/jsdoc/parser/mod.rs b/crates/oxc_semantic/src/jsdoc/parser/mod.rs index 56999c1d52fa5..65c45ff0d9e7d 100644 --- a/crates/oxc_semantic/src/jsdoc/parser/mod.rs +++ b/crates/oxc_semantic/src/jsdoc/parser/mod.rs @@ -4,3 +4,4 @@ mod parse; mod utils; pub use jsdoc::JSDoc; +pub use jsdoc_tag::JSDocTag; diff --git a/crates/oxc_semantic/src/jsdoc/parser/parse.rs b/crates/oxc_semantic/src/jsdoc/parser/parse.rs index cc159ea10bcd1..77152a93daf47 100644 --- a/crates/oxc_semantic/src/jsdoc/parser/parse.rs +++ b/crates/oxc_semantic/src/jsdoc/parser/parse.rs @@ -54,6 +54,11 @@ impl<'a> JSDocParser<'a> { let tag_name = self.take_until(|c| c == ' ' || c == '\n' || c == '@'); match tag_name { // TODO: Add more tags + "access" => self.parse_simple_tag(JSDocTagKind::Access), + "package" => self.parse_simple_tag(JSDocTagKind::Package), + "private" => self.parse_simple_tag(JSDocTagKind::Private), + "protected" => self.parse_simple_tag(JSDocTagKind::Protected), + "public" => self.parse_simple_tag(JSDocTagKind::Public), "arg" | "argument" | "param" => self.parse_parameter_tag(), "deprecated" => self.parse_simple_tag(JSDocTagKind::Deprecated), _ => self.parse_simple_tag(JSDocTagKind::Unknown(tag_name)), diff --git a/crates/oxc_semantic/src/lib.rs b/crates/oxc_semantic/src/lib.rs index 101106a142259..68e216e09c25e 100644 --- a/crates/oxc_semantic/src/lib.rs +++ b/crates/oxc_semantic/src/lib.rs @@ -19,7 +19,7 @@ pub use petgraph; pub use builder::{SemanticBuilder, SemanticBuilderReturn}; use class::ClassTable; -pub use jsdoc::JSDocFinder; +pub use jsdoc::{JSDocFinder, JSDocTag}; use oxc_ast::{ast::IdentifierReference, AstKind, TriviasMap}; use oxc_span::SourceType; pub use oxc_syntax::{ From 4cb9bfd9727b5f1b1ff360968f7e0560f7021908 Mon Sep 17 00:00:00 2001 From: Yuji Sugiura Date: Fri, 8 Mar 2024 16:42:38 +0900 Subject: [PATCH 02/10] Implement jsdoc/check-access(wip) --- crates/oxc_linter/src/rules.rs | 6 + .../src/rules/jsdoc/check_access.rs | 353 ++++++++++++++++++ .../src/snapshots/check_access.snap | 93 +++++ 3 files changed, 452 insertions(+) create mode 100644 crates/oxc_linter/src/rules/jsdoc/check_access.rs create mode 100644 crates/oxc_linter/src/snapshots/check_access.snap diff --git a/crates/oxc_linter/src/rules.rs b/crates/oxc_linter/src/rules.rs index d9212e3971d9d..7c9de12d858fb 100644 --- a/crates/oxc_linter/src/rules.rs +++ b/crates/oxc_linter/src/rules.rs @@ -329,6 +329,11 @@ mod nextjs { pub mod no_unwanted_polyfillio; } +/// https://github.com/gajus/eslint-plugin-jsdoc +mod jsdoc { + pub mod check_access; +} + oxc_macros::declare_all_lint_rules! { deepscan::bad_array_method_on_arguments, deepscan::bad_bitwise_operator, @@ -620,4 +625,5 @@ oxc_macros::declare_all_lint_rules! { nextjs::no_document_import_in_page, nextjs::no_unwanted_polyfillio, nextjs::no_before_interactive_script_outside_document, + jsdoc::check_access, } diff --git a/crates/oxc_linter/src/rules/jsdoc/check_access.rs b/crates/oxc_linter/src/rules/jsdoc/check_access.rs new file mode 100644 index 0000000000000..25fc12f10c887 --- /dev/null +++ b/crates/oxc_linter/src/rules/jsdoc/check_access.rs @@ -0,0 +1,353 @@ +use oxc_diagnostics::{ + miette::{self, Diagnostic}, + thiserror::Error, +}; +use oxc_macros::declare_oxc_lint; +use oxc_semantic::JSDocTag; +use oxc_span::Span; +use phf::phf_set; + +use crate::{context::LintContext, rule::Rule}; + +#[derive(Debug, Error, Diagnostic)] +#[error("eslint-plugin-jsdoc(check-access): TODO")] +#[diagnostic(severity(warning), help("TODO"))] +struct CheckAccessDiagnostic(#[label] pub Span); + +#[derive(Debug, Default, Clone)] +pub struct CheckAccess; + +declare_oxc_lint!( + /// ### What it does + /// TODO! + /// + /// ### Why is this bad? + /// + /// + /// ### Example + /// ```javascript + /// ``` + CheckAccess, + correctness +); + +const ACCESS_LEVELS: phf::Set<&'static str> = phf_set! { + "package", + "private", + "protected", + "public", +}; + +// TODO: Diagnostic message +// TODO: Diagnostic span, how to get it? +// TODO: Fixer? +// TODO: Check all tests are surely covered + +impl Rule for CheckAccess { + fn run_once(&self, ctx: &LintContext) { + for jsdoc in ctx.semantic().jsdoc().iter_all() { + let tags = jsdoc.tags(); + + if has_multiple_tags(tags) { + ctx.diagnostic(CheckAccessDiagnostic(Span::default())); + } + if has_invalid_access_tag(tags) { + ctx.diagnostic(CheckAccessDiagnostic(Span::default())); + } + } + } +} + +fn has_multiple_tags(tags: &[JSDocTag]) -> bool { + 1 < tags + .iter() + .map(JSDocTag::tag_name) + // TODO: Apply settings.tag_name_preference here + .filter(|tag_name| *tag_name == "access" || ACCESS_LEVELS.contains(tag_name)) + .count() +} + +fn has_invalid_access_tag(tags: &[JSDocTag]) -> bool { + // TODO: Before hand, need to update settings.rs + // https://github.com/gajus/eslint-plugin-jsdoc/blob/main/docs/settings.md + // Too many settings there and looks complicated... + // + // TODO: Apply settings.tag_name_preference here + // https://github.com/gajus/eslint-plugin-jsdoc/blob/main/docs/settings.md#alias-preference + let resolved_access_tag_name = "access"; + let access_tags = tags.iter().filter(|tag| tag.tag_name() == resolved_access_tag_name); + + for access_tag in access_tags { + if !ACCESS_LEVELS.contains(access_tag.comment.as_str()) { + return true; + } + } + + false +} + +#[test] +fn test() { + use crate::tester::Tester; + + let pass = vec![ + ( + r" + /** + * + */ + function quux (foo) { + + } + ", + None, + None, + ), + ( + r" + /** + * @access public + */ + function quux (foo) { + + } + ", + None, + None, + ), + ( + r" + /** + * @accessLevel package + */ + function quux (foo) { + + } + ", + None, + Some(serde_json::json!({ + "jsdoc": { + "tagNamePreference": { + "access": "accessLevel", + }, + }, + })), + ), + ( + r" + class MyClass { + /** + * @access private + */ + myClassField = 1 + } + ", + None, + None, + ), + ( + r" + /** + * @public + */ + function quux (foo) { + + } + ", + None, + None, + ), + ( + r" + /** + * @private + */ + function quux (foo) { + + } + ", + None, + Some(serde_json::json!({ + "jsdoc": { + "ignorePrivate": true, + }, + })), + ), + ( + r" + (function(exports, require, module, __filename, __dirname) { + // Module code actually lives in here + }); + ", + None, + None, + ), + ]; + + let fail = vec![ + ( + r" + /** + * @access foo + */ + function quux (foo) { + + } + ", + None, + None, + ), + ( + r" + /** + * @access foo + */ + function quux (foo) { + + } + ", + None, + Some(serde_json::json!({ + "jsdoc": { + "ignorePrivate": true, + }, + })), + ), + // ( + // r" + // /** + // * @accessLevel foo + // */ + // function quux (foo) { + + // } + // ", + // None, + // Some(serde_json::json!({ + // "jsdoc": { + // "tagNamePreference": { + // "access": "accessLevel", + // }, + // }, + // })), + // ), + ( + r" + /** + * @access + */ + function quux (foo) { + + } + ", + None, + Some(serde_json::json!({ + "jsdoc": { + "tagNamePreference": { + "access": false, + }, + }, + })), + ), + ( + r" + class MyClass { + /** + * @access + */ + myClassField = 1 + } + ", + None, + None, + ), + ( + r" + /** + * @access public + * @public + */ + function quux (foo) { + + } + ", + None, + None, + ), + ( + r" + /** + * @access public + * @access private + */ + function quux (foo) { + + } + ", + None, + None, + ), + ( + r" + /** + * @access public + * @access private + */ + function quux (foo) { + + } + ", + None, + Some(serde_json::json!({ + "jsdoc": { + "ignorePrivate": true, + }, + })), + ), + ( + r" + /** + * @public + * @private + */ + function quux (foo) { + + } + ", + None, + None, + ), + ( + r" + /** + * @public + * @private + */ + function quux (foo) { + + } + ", + None, + Some(serde_json::json!({ + "jsdoc": { + "ignorePrivate": true, + }, + })), + ), + ( + r" + /** + * @public + * @public + */ + function quux (foo) { + + } + ", + None, + None, + ), + ]; + + Tester::new(CheckAccess::NAME, pass, fail).test_and_snapshot(); +} diff --git a/crates/oxc_linter/src/snapshots/check_access.snap b/crates/oxc_linter/src/snapshots/check_access.snap new file mode 100644 index 0000000000000..dbaa0ed3de873 --- /dev/null +++ b/crates/oxc_linter/src/snapshots/check_access.snap @@ -0,0 +1,93 @@ +--- +source: crates/oxc_linter/src/tester.rs +expression: check_access +--- + ⚠ eslint-plugin-jsdoc(check-access): TODO + ╭─[check_access.tsx:1:1] + 1 │ + · ▲ + 2 │ /** + 3 │ * @access foo + ╰──── + help: TODO + + ⚠ eslint-plugin-jsdoc(check-access): TODO + ╭─[check_access.tsx:1:1] + 1 │ + · ▲ + 2 │ /** + 3 │ * @access foo + ╰──── + help: TODO + + ⚠ eslint-plugin-jsdoc(check-access): TODO + ╭─[check_access.tsx:1:1] + 1 │ + · ▲ + 2 │ /** + 3 │ * @access + ╰──── + help: TODO + + ⚠ eslint-plugin-jsdoc(check-access): TODO + ╭─[check_access.tsx:1:1] + 1 │ + · ▲ + 2 │ class MyClass { + 3 │ /** + ╰──── + help: TODO + + ⚠ eslint-plugin-jsdoc(check-access): TODO + ╭─[check_access.tsx:1:1] + 1 │ + · ▲ + 2 │ /** + 3 │ * @access public + ╰──── + help: TODO + + ⚠ eslint-plugin-jsdoc(check-access): TODO + ╭─[check_access.tsx:1:1] + 1 │ + · ▲ + 2 │ /** + 3 │ * @access public + ╰──── + help: TODO + + ⚠ eslint-plugin-jsdoc(check-access): TODO + ╭─[check_access.tsx:1:1] + 1 │ + · ▲ + 2 │ /** + 3 │ * @access public + ╰──── + help: TODO + + ⚠ eslint-plugin-jsdoc(check-access): TODO + ╭─[check_access.tsx:1:1] + 1 │ + · ▲ + 2 │ /** + 3 │ * @public + ╰──── + help: TODO + + ⚠ eslint-plugin-jsdoc(check-access): TODO + ╭─[check_access.tsx:1:1] + 1 │ + · ▲ + 2 │ /** + 3 │ * @public + ╰──── + help: TODO + + ⚠ eslint-plugin-jsdoc(check-access): TODO + ╭─[check_access.tsx:1:1] + 1 │ + · ▲ + 2 │ /** + 3 │ * @public + ╰──── + help: TODO From f337e71a549c4198964d7aecad94f20f75b2d887 Mon Sep 17 00:00:00 2001 From: Yuji Sugiura Date: Fri, 8 Mar 2024 16:50:42 +0900 Subject: [PATCH 03/10] Fix CI --- crates/oxc_linter/src/rules.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/oxc_linter/src/rules.rs b/crates/oxc_linter/src/rules.rs index 7c9de12d858fb..e41174a2a3e7b 100644 --- a/crates/oxc_linter/src/rules.rs +++ b/crates/oxc_linter/src/rules.rs @@ -329,7 +329,7 @@ mod nextjs { pub mod no_unwanted_polyfillio; } -/// https://github.com/gajus/eslint-plugin-jsdoc +/// mod jsdoc { pub mod check_access; } From 6d43ebc49cf823d7716164060f68f71309d94292 Mon Sep 17 00:00:00 2001 From: Yuji Sugiura Date: Fri, 15 Mar 2024 13:16:22 +0900 Subject: [PATCH 04/10] Update --- .../src/rules/jsdoc/check_access.rs | 93 ++++++++----------- .../src/snapshots/check_access.snap | 9 ++ 2 files changed, 50 insertions(+), 52 deletions(-) diff --git a/crates/oxc_linter/src/rules/jsdoc/check_access.rs b/crates/oxc_linter/src/rules/jsdoc/check_access.rs index 25fc12f10c887..e1d8f6c5bcc70 100644 --- a/crates/oxc_linter/src/rules/jsdoc/check_access.rs +++ b/crates/oxc_linter/src/rules/jsdoc/check_access.rs @@ -3,9 +3,9 @@ use oxc_diagnostics::{ thiserror::Error, }; use oxc_macros::declare_oxc_lint; -use oxc_semantic::JSDocTag; use oxc_span::Span; use phf::phf_set; +use rustc_hash::FxHashSet; use crate::{context::LintContext, rule::Rule}; @@ -40,50 +40,39 @@ const ACCESS_LEVELS: phf::Set<&'static str> = phf_set! { // TODO: Diagnostic message // TODO: Diagnostic span, how to get it? -// TODO: Fixer? -// TODO: Check all tests are surely covered impl Rule for CheckAccess { fn run_once(&self, ctx: &LintContext) { - for jsdoc in ctx.semantic().jsdoc().iter_all() { - let tags = jsdoc.tags(); + let settings = &ctx.settings().jsdoc; + let resolved_access_tag_name = settings.resolve_tag_name("access"); - if has_multiple_tags(tags) { - ctx.diagnostic(CheckAccessDiagnostic(Span::default())); - } - if has_invalid_access_tag(tags) { - ctx.diagnostic(CheckAccessDiagnostic(Span::default())); - } + let mut access_related_tag_names = FxHashSet::default(); + access_related_tag_names.insert(resolved_access_tag_name.to_string()); + for level in &ACCESS_LEVELS { + access_related_tag_names.insert(settings.resolve_tag_name(level)); } - } -} -fn has_multiple_tags(tags: &[JSDocTag]) -> bool { - 1 < tags - .iter() - .map(JSDocTag::tag_name) - // TODO: Apply settings.tag_name_preference here - .filter(|tag_name| *tag_name == "access" || ACCESS_LEVELS.contains(tag_name)) - .count() -} + for jsdoc in ctx.semantic().jsdoc().iter_all() { + let mut access_related_tags_count = 0; + for tag in jsdoc.tags() { + let tag_name = tag.tag_name(); + + if access_related_tag_names.contains(tag_name) { + access_related_tags_count += 1; + } -fn has_invalid_access_tag(tags: &[JSDocTag]) -> bool { - // TODO: Before hand, need to update settings.rs - // https://github.com/gajus/eslint-plugin-jsdoc/blob/main/docs/settings.md - // Too many settings there and looks complicated... - // - // TODO: Apply settings.tag_name_preference here - // https://github.com/gajus/eslint-plugin-jsdoc/blob/main/docs/settings.md#alias-preference - let resolved_access_tag_name = "access"; - let access_tags = tags.iter().filter(|tag| tag.tag_name() == resolved_access_tag_name); + // Has valid access level? + if tag_name == resolved_access_tag_name && !ACCESS_LEVELS.contains(&tag.comment) { + ctx.diagnostic(CheckAccessDiagnostic(Span::default())); + } - for access_tag in access_tags { - if !ACCESS_LEVELS.contains(access_tag.comment.as_str()) { - return true; + // Has redundant access level? + if 1 < access_related_tags_count { + ctx.diagnostic(CheckAccessDiagnostic(Span::default())); + } + } } } - - false } #[test] @@ -213,24 +202,24 @@ fn test() { }, })), ), - // ( - // r" - // /** - // * @accessLevel foo - // */ - // function quux (foo) { + ( + r" + /** + * @accessLevel foo + */ + function quux (foo) { - // } - // ", - // None, - // Some(serde_json::json!({ - // "jsdoc": { - // "tagNamePreference": { - // "access": "accessLevel", - // }, - // }, - // })), - // ), + } + ", + None, + Some(serde_json::json!({ + "jsdoc": { + "tagNamePreference": { + "access": "accessLevel", + }, + }, + })), + ), ( r" /** diff --git a/crates/oxc_linter/src/snapshots/check_access.snap b/crates/oxc_linter/src/snapshots/check_access.snap index dbaa0ed3de873..ae00279ffe03f 100644 --- a/crates/oxc_linter/src/snapshots/check_access.snap +++ b/crates/oxc_linter/src/snapshots/check_access.snap @@ -20,6 +20,15 @@ expression: check_access ╰──── help: TODO + ⚠ eslint-plugin-jsdoc(check-access): TODO + ╭─[check_access.tsx:1:1] + 1 │ + · ▲ + 2 │ /** + 3 │ * @accessLevel foo + ╰──── + help: TODO + ⚠ eslint-plugin-jsdoc(check-access): TODO ╭─[check_access.tsx:1:1] 1 │ From f3c432263cdff27c099e5789eb671f6c0410ffe4 Mon Sep 17 00:00:00 2001 From: Yuji Sugiura Date: Thu, 28 Mar 2024 10:00:51 +0900 Subject: [PATCH 05/10] Follow main updates --- .../src/rules/jsdoc/check_access.rs | 13 ++- .../src/snapshots/check_access.snap | 80 +++++++++---------- 2 files changed, 45 insertions(+), 48 deletions(-) diff --git a/crates/oxc_linter/src/rules/jsdoc/check_access.rs b/crates/oxc_linter/src/rules/jsdoc/check_access.rs index e1d8f6c5bcc70..6d1879187471b 100644 --- a/crates/oxc_linter/src/rules/jsdoc/check_access.rs +++ b/crates/oxc_linter/src/rules/jsdoc/check_access.rs @@ -39,7 +39,6 @@ const ACCESS_LEVELS: phf::Set<&'static str> = phf_set! { }; // TODO: Diagnostic message -// TODO: Diagnostic span, how to get it? impl Rule for CheckAccess { fn run_once(&self, ctx: &LintContext) { @@ -54,21 +53,19 @@ impl Rule for CheckAccess { for jsdoc in ctx.semantic().jsdoc().iter_all() { let mut access_related_tags_count = 0; - for tag in jsdoc.tags() { - let tag_name = tag.tag_name(); - - if access_related_tag_names.contains(tag_name) { + for (span, tag) in jsdoc.tags() { + if access_related_tag_names.contains(tag.kind) { access_related_tags_count += 1; } // Has valid access level? - if tag_name == resolved_access_tag_name && !ACCESS_LEVELS.contains(&tag.comment) { - ctx.diagnostic(CheckAccessDiagnostic(Span::default())); + if tag.kind == resolved_access_tag_name && !ACCESS_LEVELS.contains(&tag.comment()) { + ctx.diagnostic(CheckAccessDiagnostic(*span)); } // Has redundant access level? if 1 < access_related_tags_count { - ctx.diagnostic(CheckAccessDiagnostic(Span::default())); + ctx.diagnostic(CheckAccessDiagnostic(*span)); } } } diff --git a/crates/oxc_linter/src/snapshots/check_access.snap b/crates/oxc_linter/src/snapshots/check_access.snap index ae00279ffe03f..4bc4c2e9d301d 100644 --- a/crates/oxc_linter/src/snapshots/check_access.snap +++ b/crates/oxc_linter/src/snapshots/check_access.snap @@ -3,100 +3,100 @@ source: crates/oxc_linter/src/tester.rs expression: check_access --- ⚠ eslint-plugin-jsdoc(check-access): TODO - ╭─[check_access.tsx:1:1] - 1 │ - · ▲ + ╭─[check_access.tsx:3:17] 2 │ /** 3 │ * @access foo + · ─────── + 4 │ */ ╰──── help: TODO ⚠ eslint-plugin-jsdoc(check-access): TODO - ╭─[check_access.tsx:1:1] - 1 │ - · ▲ + ╭─[check_access.tsx:3:17] 2 │ /** 3 │ * @access foo + · ─────── + 4 │ */ ╰──── help: TODO ⚠ eslint-plugin-jsdoc(check-access): TODO - ╭─[check_access.tsx:1:1] - 1 │ - · ▲ + ╭─[check_access.tsx:3:25] 2 │ /** 3 │ * @accessLevel foo + · ──────────── + 4 │ */ ╰──── help: TODO ⚠ eslint-plugin-jsdoc(check-access): TODO - ╭─[check_access.tsx:1:1] - 1 │ - · ▲ + ╭─[check_access.tsx:3:17] 2 │ /** 3 │ * @access + · ─────── + 4 │ */ ╰──── help: TODO ⚠ eslint-plugin-jsdoc(check-access): TODO - ╭─[check_access.tsx:1:1] - 1 │ - · ▲ - 2 │ class MyClass { + ╭─[check_access.tsx:4:15] 3 │ /** + 4 │ * @access + · ─────── + 5 │ */ ╰──── help: TODO ⚠ eslint-plugin-jsdoc(check-access): TODO - ╭─[check_access.tsx:1:1] - 1 │ - · ▲ - 2 │ /** + ╭─[check_access.tsx:4:17] 3 │ * @access public + 4 │ * @public + · ─────── + 5 │ */ ╰──── help: TODO ⚠ eslint-plugin-jsdoc(check-access): TODO - ╭─[check_access.tsx:1:1] - 1 │ - · ▲ - 2 │ /** + ╭─[check_access.tsx:4:17] 3 │ * @access public + 4 │ * @access private + · ─────── + 5 │ */ ╰──── help: TODO ⚠ eslint-plugin-jsdoc(check-access): TODO - ╭─[check_access.tsx:1:1] - 1 │ - · ▲ - 2 │ /** + ╭─[check_access.tsx:4:17] 3 │ * @access public + 4 │ * @access private + · ─────── + 5 │ */ ╰──── help: TODO ⚠ eslint-plugin-jsdoc(check-access): TODO - ╭─[check_access.tsx:1:1] - 1 │ - · ▲ - 2 │ /** + ╭─[check_access.tsx:4:17] 3 │ * @public + 4 │ * @private + · ──────── + 5 │ */ ╰──── help: TODO ⚠ eslint-plugin-jsdoc(check-access): TODO - ╭─[check_access.tsx:1:1] - 1 │ - · ▲ - 2 │ /** + ╭─[check_access.tsx:4:17] 3 │ * @public + 4 │ * @private + · ──────── + 5 │ */ ╰──── help: TODO ⚠ eslint-plugin-jsdoc(check-access): TODO - ╭─[check_access.tsx:1:1] - 1 │ - · ▲ - 2 │ /** + ╭─[check_access.tsx:4:17] 3 │ * @public + 4 │ * @public + · ─────── + 5 │ */ ╰──── help: TODO From 89fc42e361227a1943d0c1b3087e2cf9496c9936 Mon Sep 17 00:00:00 2001 From: Yuji Sugiura Date: Thu, 28 Mar 2024 10:08:27 +0900 Subject: [PATCH 06/10] Fix exports --- crates/oxc_semantic/src/jsdoc/mod.rs | 1 + crates/oxc_semantic/src/jsdoc/parser/jsdoc.rs | 3 --- crates/oxc_semantic/src/lib.rs | 2 +- 3 files changed, 2 insertions(+), 4 deletions(-) diff --git a/crates/oxc_semantic/src/jsdoc/mod.rs b/crates/oxc_semantic/src/jsdoc/mod.rs index 6a6dde05b9e6f..801a5e87fc8f1 100644 --- a/crates/oxc_semantic/src/jsdoc/mod.rs +++ b/crates/oxc_semantic/src/jsdoc/mod.rs @@ -4,4 +4,5 @@ mod parser; pub use builder::JSDocBuilder; pub use finder::JSDocFinder; +pub use parser::JSDoc; pub use parser::JSDocTag; diff --git a/crates/oxc_semantic/src/jsdoc/parser/jsdoc.rs b/crates/oxc_semantic/src/jsdoc/parser/jsdoc.rs index 221fd805f3d4b..45658210af940 100644 --- a/crates/oxc_semantic/src/jsdoc/parser/jsdoc.rs +++ b/crates/oxc_semantic/src/jsdoc/parser/jsdoc.rs @@ -121,7 +121,4 @@ line let (span, _) = tags.next().unwrap(); assert_eq!(span.source_text(semantic.source_text), "@k4"); } - - // Should implement here...? Or leave it to usecases? - // fn has_deprecated_tag(&self) -> bool {} } diff --git a/crates/oxc_semantic/src/lib.rs b/crates/oxc_semantic/src/lib.rs index 68f85c1e425e6..e795a3606f3c6 100644 --- a/crates/oxc_semantic/src/lib.rs +++ b/crates/oxc_semantic/src/lib.rs @@ -19,7 +19,7 @@ pub use petgraph; pub use builder::{SemanticBuilder, SemanticBuilderReturn}; use class::ClassTable; -pub use jsdoc::{JSDocFinder, JSDocTag}; +pub use jsdoc::{JSDocFinder, JSDoc, JSDocTag}; use oxc_ast::{ast::IdentifierReference, AstKind, Trivias}; use oxc_span::SourceType; pub use oxc_syntax::{ From 9be38e01243c949c59d02af92b813dd37a21c225 Mon Sep 17 00:00:00 2001 From: Yuji Sugiura Date: Thu, 28 Mar 2024 10:09:23 +0900 Subject: [PATCH 07/10] Format --- crates/oxc_semantic/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/oxc_semantic/src/lib.rs b/crates/oxc_semantic/src/lib.rs index e795a3606f3c6..d7f3182c9a5da 100644 --- a/crates/oxc_semantic/src/lib.rs +++ b/crates/oxc_semantic/src/lib.rs @@ -19,7 +19,7 @@ pub use petgraph; pub use builder::{SemanticBuilder, SemanticBuilderReturn}; use class::ClassTable; -pub use jsdoc::{JSDocFinder, JSDoc, JSDocTag}; +pub use jsdoc::{JSDoc, JSDocFinder, JSDocTag}; use oxc_ast::{ast::IdentifierReference, AstKind, Trivias}; use oxc_span::SourceType; pub use oxc_syntax::{ From 36d3b7666f388c863736e9d0a00aa9d88f20a43a Mon Sep 17 00:00:00 2001 From: Yuji Sugiura Date: Mon, 1 Apr 2024 14:13:35 +0900 Subject: [PATCH 08/10] Fix messages --- .../src/rules/jsdoc/check_access.rs | 44 ++++++++++++++----- .../src/snapshots/check_access.snap | 44 +++++++++---------- 2 files changed, 56 insertions(+), 32 deletions(-) diff --git a/crates/oxc_linter/src/rules/jsdoc/check_access.rs b/crates/oxc_linter/src/rules/jsdoc/check_access.rs index 6d1879187471b..c4077170cae8e 100644 --- a/crates/oxc_linter/src/rules/jsdoc/check_access.rs +++ b/crates/oxc_linter/src/rules/jsdoc/check_access.rs @@ -10,25 +10,51 @@ use rustc_hash::FxHashSet; use crate::{context::LintContext, rule::Rule}; #[derive(Debug, Error, Diagnostic)] -#[error("eslint-plugin-jsdoc(check-access): TODO")] -#[diagnostic(severity(warning), help("TODO"))] -struct CheckAccessDiagnostic(#[label] pub Span); +enum CheckAccessDiagnostic { + #[error("eslint-plugin-jsdoc(check-access): Invalid access level is specified.")] + #[diagnostic( + severity(warning), + help("Valid access leves are `package`, `private`, `protected`, and `public`.") + )] + InvalidAccessLevel(#[label] Span), + + #[error("eslint-plugin-jsdoc(check-access): Mixing of @access with @public, @private, @protected, or @package on the same doc block.")] + #[diagnostic( + severity(warning), + help("There should be only one instance of access tag in a JSDoc comment.") + )] + RedundantAccessTags(#[label] Span), +} #[derive(Debug, Default, Clone)] pub struct CheckAccess; declare_oxc_lint!( /// ### What it does - /// TODO! + /// Checks that `@access` tags use one of the following values: + /// - "package", "private", "protected", "public" /// - /// ### Why is this bad? + /// Also reports: + /// - Mixing of `@access` with `@public`, `@private`, `@protected`, or `@package` on the same doc block. + /// - Use of multiple instances of `@access` (or the `@public`, etc) on the same doc block. /// + /// ### Why is this bad? + /// It is important to have a consistent way of specifying access levels. /// /// ### Example /// ```javascript + /// // Passing + /// /** @access private */ + /// + /// /** @private */ + /// + /// // Failing + /// /** @access private @public */ + /// + /// /** @access invalidlevel */ /// ``` CheckAccess, - correctness + restriction ); const ACCESS_LEVELS: phf::Set<&'static str> = phf_set! { @@ -38,8 +64,6 @@ const ACCESS_LEVELS: phf::Set<&'static str> = phf_set! { "public", }; -// TODO: Diagnostic message - impl Rule for CheckAccess { fn run_once(&self, ctx: &LintContext) { let settings = &ctx.settings().jsdoc; @@ -60,12 +84,12 @@ impl Rule for CheckAccess { // Has valid access level? if tag.kind == resolved_access_tag_name && !ACCESS_LEVELS.contains(&tag.comment()) { - ctx.diagnostic(CheckAccessDiagnostic(*span)); + ctx.diagnostic(CheckAccessDiagnostic::InvalidAccessLevel(*span)); } // Has redundant access level? if 1 < access_related_tags_count { - ctx.diagnostic(CheckAccessDiagnostic(*span)); + ctx.diagnostic(CheckAccessDiagnostic::RedundantAccessTags(*span)); } } } diff --git a/crates/oxc_linter/src/snapshots/check_access.snap b/crates/oxc_linter/src/snapshots/check_access.snap index 4bc4c2e9d301d..050349d7bd6b5 100644 --- a/crates/oxc_linter/src/snapshots/check_access.snap +++ b/crates/oxc_linter/src/snapshots/check_access.snap @@ -2,101 +2,101 @@ source: crates/oxc_linter/src/tester.rs expression: check_access --- - ⚠ eslint-plugin-jsdoc(check-access): TODO + ⚠ eslint-plugin-jsdoc(check-access): Invalid access level is specified. ╭─[check_access.tsx:3:17] 2 │ /** 3 │ * @access foo · ─────── 4 │ */ ╰──── - help: TODO + help: Valid access leves are `package`, `private`, `protected`, and `public`. - ⚠ eslint-plugin-jsdoc(check-access): TODO + ⚠ eslint-plugin-jsdoc(check-access): Invalid access level is specified. ╭─[check_access.tsx:3:17] 2 │ /** 3 │ * @access foo · ─────── 4 │ */ ╰──── - help: TODO + help: Valid access leves are `package`, `private`, `protected`, and `public`. - ⚠ eslint-plugin-jsdoc(check-access): TODO + ⚠ eslint-plugin-jsdoc(check-access): Invalid access level is specified. ╭─[check_access.tsx:3:25] 2 │ /** 3 │ * @accessLevel foo · ──────────── 4 │ */ ╰──── - help: TODO + help: Valid access leves are `package`, `private`, `protected`, and `public`. - ⚠ eslint-plugin-jsdoc(check-access): TODO + ⚠ eslint-plugin-jsdoc(check-access): Invalid access level is specified. ╭─[check_access.tsx:3:17] 2 │ /** 3 │ * @access · ─────── 4 │ */ ╰──── - help: TODO + help: Valid access leves are `package`, `private`, `protected`, and `public`. - ⚠ eslint-plugin-jsdoc(check-access): TODO + ⚠ eslint-plugin-jsdoc(check-access): Invalid access level is specified. ╭─[check_access.tsx:4:15] 3 │ /** 4 │ * @access · ─────── 5 │ */ ╰──── - help: TODO + help: Valid access leves are `package`, `private`, `protected`, and `public`. - ⚠ eslint-plugin-jsdoc(check-access): TODO + ⚠ eslint-plugin-jsdoc(check-access): Mixing of @access with @public, @private, @protected, or @package on the same doc block. ╭─[check_access.tsx:4:17] 3 │ * @access public 4 │ * @public · ─────── 5 │ */ ╰──── - help: TODO + help: There should be only one instance of access tag in a JSDoc comment. - ⚠ eslint-plugin-jsdoc(check-access): TODO + ⚠ eslint-plugin-jsdoc(check-access): Mixing of @access with @public, @private, @protected, or @package on the same doc block. ╭─[check_access.tsx:4:17] 3 │ * @access public 4 │ * @access private · ─────── 5 │ */ ╰──── - help: TODO + help: There should be only one instance of access tag in a JSDoc comment. - ⚠ eslint-plugin-jsdoc(check-access): TODO + ⚠ eslint-plugin-jsdoc(check-access): Mixing of @access with @public, @private, @protected, or @package on the same doc block. ╭─[check_access.tsx:4:17] 3 │ * @access public 4 │ * @access private · ─────── 5 │ */ ╰──── - help: TODO + help: There should be only one instance of access tag in a JSDoc comment. - ⚠ eslint-plugin-jsdoc(check-access): TODO + ⚠ eslint-plugin-jsdoc(check-access): Mixing of @access with @public, @private, @protected, or @package on the same doc block. ╭─[check_access.tsx:4:17] 3 │ * @public 4 │ * @private · ──────── 5 │ */ ╰──── - help: TODO + help: There should be only one instance of access tag in a JSDoc comment. - ⚠ eslint-plugin-jsdoc(check-access): TODO + ⚠ eslint-plugin-jsdoc(check-access): Mixing of @access with @public, @private, @protected, or @package on the same doc block. ╭─[check_access.tsx:4:17] 3 │ * @public 4 │ * @private · ──────── 5 │ */ ╰──── - help: TODO + help: There should be only one instance of access tag in a JSDoc comment. - ⚠ eslint-plugin-jsdoc(check-access): TODO + ⚠ eslint-plugin-jsdoc(check-access): Mixing of @access with @public, @private, @protected, or @package on the same doc block. ╭─[check_access.tsx:4:17] 3 │ * @public 4 │ * @public · ─────── 5 │ */ ╰──── - help: TODO + help: There should be only one instance of access tag in a JSDoc comment. From e5840e1d5a3c5b1aa93d6fed73c97d935a10ec7c Mon Sep 17 00:00:00 2001 From: Yuji Sugiura Date: Mon, 1 Apr 2024 14:16:26 +0900 Subject: [PATCH 09/10] Fix typo --- crates/oxc_linter/src/rules/jsdoc/check_access.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/oxc_linter/src/rules/jsdoc/check_access.rs b/crates/oxc_linter/src/rules/jsdoc/check_access.rs index c4077170cae8e..ade094f0e9aa6 100644 --- a/crates/oxc_linter/src/rules/jsdoc/check_access.rs +++ b/crates/oxc_linter/src/rules/jsdoc/check_access.rs @@ -14,7 +14,7 @@ enum CheckAccessDiagnostic { #[error("eslint-plugin-jsdoc(check-access): Invalid access level is specified.")] #[diagnostic( severity(warning), - help("Valid access leves are `package`, `private`, `protected`, and `public`.") + help("Valid access levels are `package`, `private`, `protected`, and `public`.") )] InvalidAccessLevel(#[label] Span), From 1e93e27eebf2f5b54f27e8a90b4f7ebd9de8a82e Mon Sep 17 00:00:00 2001 From: Yuji Sugiura Date: Mon, 1 Apr 2024 14:20:09 +0900 Subject: [PATCH 10/10] Update snapshot --- crates/oxc_linter/src/snapshots/check_access.snap | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/crates/oxc_linter/src/snapshots/check_access.snap b/crates/oxc_linter/src/snapshots/check_access.snap index 050349d7bd6b5..dcb89a397fffa 100644 --- a/crates/oxc_linter/src/snapshots/check_access.snap +++ b/crates/oxc_linter/src/snapshots/check_access.snap @@ -9,7 +9,7 @@ expression: check_access · ─────── 4 │ */ ╰──── - help: Valid access leves are `package`, `private`, `protected`, and `public`. + help: Valid access levels are `package`, `private`, `protected`, and `public`. ⚠ eslint-plugin-jsdoc(check-access): Invalid access level is specified. ╭─[check_access.tsx:3:17] @@ -18,7 +18,7 @@ expression: check_access · ─────── 4 │ */ ╰──── - help: Valid access leves are `package`, `private`, `protected`, and `public`. + help: Valid access levels are `package`, `private`, `protected`, and `public`. ⚠ eslint-plugin-jsdoc(check-access): Invalid access level is specified. ╭─[check_access.tsx:3:25] @@ -27,7 +27,7 @@ expression: check_access · ──────────── 4 │ */ ╰──── - help: Valid access leves are `package`, `private`, `protected`, and `public`. + help: Valid access levels are `package`, `private`, `protected`, and `public`. ⚠ eslint-plugin-jsdoc(check-access): Invalid access level is specified. ╭─[check_access.tsx:3:17] @@ -36,7 +36,7 @@ expression: check_access · ─────── 4 │ */ ╰──── - help: Valid access leves are `package`, `private`, `protected`, and `public`. + help: Valid access levels are `package`, `private`, `protected`, and `public`. ⚠ eslint-plugin-jsdoc(check-access): Invalid access level is specified. ╭─[check_access.tsx:4:15] @@ -45,7 +45,7 @@ expression: check_access · ─────── 5 │ */ ╰──── - help: Valid access leves are `package`, `private`, `protected`, and `public`. + help: Valid access levels are `package`, `private`, `protected`, and `public`. ⚠ eslint-plugin-jsdoc(check-access): Mixing of @access with @public, @private, @protected, or @package on the same doc block. ╭─[check_access.tsx:4:17]