dan/oxc
1
0
Fork 0
mirror of https://github.com/danbulant/oxc synced 2026-05-19 12:19:15 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions

refactor(linter/jsdoc): Misc improvements (#3109)

Browse source 
- [x] Always use resolved_tag_name
- [x] Nits syntax, naming fixes
- [x] Split up utils
This commit is contained in:
Yuji Sugiura 2024-04-27 22:47:33 +09:00 committed by GitHub
parent 17131d23b8
commit 1f12aee149
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
12 changed files with 112 additions and 108 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

9
crates/oxc_linter/src/ast_util.rs
View file

@ -413,3 +413,12 @@ pub fn is_global_require_call(call_expr: &CallExpression, ctx: &LintContext) ->
false
}
}
pub fn is_function_node(node: &AstNode) -> bool {
match node.kind() {
AstKind::Function(f) if f.is_function_declaration() => true,
AstKind::Function(f) if f.is_expression() => true,
AstKind::ArrowFunctionExpression(_) => true,
_ => false,
}
}

9
crates/oxc_linter/src/rules/jsdoc/check_access.rs
View file

@ -78,14 +78,17 @@ 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 kind = tag.kind.parsed();
if access_related_tag_names.contains(kind) {
let tag_name = tag.kind.parsed();
if access_related_tag_names.contains(tag_name) {
access_related_tags_count += 1;
}
// Has valid access level?
let comment = tag.comment();
if kind == resolved_access_tag_name && !ACCESS_LEVELS.contains(&comment.parsed()) {
if tag_name == resolved_access_tag_name
&& !ACCESS_LEVELS.contains(&comment.parsed())
{
ctx.diagnostic(CheckAccessDiagnostic::InvalidAccessLevel(
comment.span_trimmed_first_line(),
));

3
crates/oxc_linter/src/rules/jsdoc/check_property_names.rs
View file

@ -69,8 +69,7 @@ impl Rule for CheckPropertyNames {
if tag.kind.parsed() != resolved_property_tag_name {
continue;
}
let (_, name_part, _) = tag.type_name_comment();
let Some(name_part) = name_part else {
let (_, Some(name_part), _) = tag.type_name_comment() else {
continue;
};

14
crates/oxc_linter/src/rules/jsdoc/empty_tags.rs
View file

@ -95,11 +95,11 @@ impl Rule for EmptyTags {
}
fn run_once(&self, ctx: &LintContext) {
let is_empty_tag_kind = |kind: &str| {
if EMPTY_TAGS.contains(kind) {
let is_empty_tag_kind = |tag_name: &str| {
if EMPTY_TAGS.contains(tag_name) {
return true;
}
if !self.0.tags.is_empty() && self.0.tags.contains(&kind.to_string()) {
if !self.0.tags.is_empty() && self.0.tags.contains(&tag_name.to_string()) {
return true;
}
false
@ -107,10 +107,12 @@ impl Rule for EmptyTags {
for jsdoc in ctx.semantic().jsdoc().iter_all() {
for tag in jsdoc.tags() {
let kind = tag.kind.parsed();
if !is_empty_tag_kind(kind) {
let tag_name = tag.kind.parsed();
if !is_empty_tag_kind(tag_name) {
continue;
}
let comment = tag.comment();
if comment.parsed().is_empty() {
continue;
@ -118,7 +120,7 @@ impl Rule for EmptyTags {
ctx.diagnostic(EmptyTagsDiagnostic(
comment.span_trimmed_first_line(),
kind.to_string(),
tag_name.to_string(),
));
}
}

54
crates/oxc_linter/src/rules/jsdoc/implements_on_classes.rs
View file

@ -1,3 +1,7 @@
use crate::{
ast_util::is_function_node, context::LintContext, rule::Rule,
utils::get_function_nearest_jsdoc_node, AstNode,
};
use oxc_ast::AstKind;
use oxc_diagnostics::{
miette::{self, Diagnostic},
@ -6,8 +10,6 @@ use oxc_diagnostics::{
use oxc_macros::declare_oxc_lint;
use oxc_span::Span;
use crate::{context::LintContext, rule::Rule, AstNode};
#[derive(Debug, Error, Diagnostic)]
#[error(
"eslint-plugin-jsdoc(implements-on-classes): `@implements` used on a non-constructor function"
@ -51,55 +53,41 @@ declare_oxc_lint!(
correctness
);
/// Get the definition root node of a function.
/// JSDoc often appears on the parent node of a function.
///
/// ```js
/// /** FunctionDeclaration */
/// function foo() {}
///
/// /** VariableDeclaration > VariableDeclarator > FunctionExpression */
/// const bar = function() {}
///
/// /** VariableDeclaration > VariableDeclarator > ArrowFunctionExpression */
/// const baz = () => {}
/// ```
fn get_function_definition_node<'a, 'b>(
node: &'b AstNode<'a>,
ctx: &'b LintContext<'a>,
) -> Option<&'b AstNode<'a>> {
match node.kind() {
AstKind::Function(f) if f.is_function_declaration() => return Some(node),
AstKind::Function(f) if f.is_expression() => {}
AstKind::ArrowFunctionExpression(_) => {}
_ => return None,
};
fn is_function_inside_of_class<'a, 'b>(node: &'b AstNode<'a>, ctx: &'b LintContext<'a>) -> bool {
let mut current_node = node;
while let Some(parent_node) = ctx.nodes().parent_node(current_node.id()) {
match parent_node.kind() {
// `MethodDefinition` is not a target
AstKind::VariableDeclarator(_) | AstKind::ParenthesizedExpression(_) => {
AstKind::MethodDefinition(_) | AstKind::PropertyDefinition(_) => return true,
// Keep looking up only if the node is wrapped by `()`
AstKind::ParenthesizedExpression(_) => {
current_node = parent_node;
}
AstKind::VariableDeclaration(_) => return Some(parent_node),
_ => return None,
_ => return false,
}
}
None
false
}
impl Rule for ImplementsOnClasses {
fn run<'a>(&self, node: &AstNode<'a>, ctx: &LintContext<'a>) {
let Some(jsdocs) = get_function_definition_node(node, ctx)
if !is_function_node(node) {
return;
}
// Filter plain declared (arrow) function.
// I'm not sure but this rule does not care node like `MethodDefinition`.
if is_function_inside_of_class(node, ctx) {
return;
}
let Some(jsdocs) = get_function_nearest_jsdoc_node(node, ctx)
.and_then(|node| ctx.jsdoc().get_all_by_node(node))
else {
return;
};
let settings = &ctx.settings().jsdoc;
let resolved_implements_tag_name = settings.resolve_tag_name("implements");
let resolved_class_tag_name = settings.resolve_tag_name("class");
let resolved_constructor_tag_name = settings.resolve_tag_name("constructor");

58
crates/oxc_linter/src/rules/jsdoc/no_defaults.rs
View file

@ -1,4 +1,3 @@
use oxc_ast::AstKind;
use oxc_diagnostics::{
miette::{self, Diagnostic},
thiserror::Error,
@ -7,7 +6,10 @@ use oxc_macros::declare_oxc_lint;
use oxc_span::Span;
use serde::Deserialize;
use crate::{context::LintContext, rule::Rule, AstNode};
use crate::{
ast_util::is_function_node, context::LintContext, rule::Rule,
utils::get_function_nearest_jsdoc_node, AstNode,
};
#[derive(Debug, Error, Diagnostic)]
#[error("eslint-plugin-jsdoc(no-defaults): Defaults are not permitted.")]
@ -48,52 +50,6 @@ struct NoDefaultsConfig {
no_optional_param_names: bool,
}
/// Get the definition root node of a function.
/// JSDoc often appears on the parent node of a function.
///
/// ```js
/// /** FunctionDeclaration */
/// function foo() {}
///
/// /** VariableDeclaration > VariableDeclarator > FunctionExpression */
/// const bar = function() {}
///
/// /** VariableDeclaration > VariableDeclarator > ArrowFunctionExpression */
/// const baz = () => {}
///
/// /** MethodDefinition > FunctionExpression */
/// class X { quux() {} }
///
/// /** PropertyDefinition > ArrowFunctionExpression */
/// class X { quux = () => {} }
/// ```
fn get_function_definition_node<'a, 'b>(
node: &'b AstNode<'a>,
ctx: &'b LintContext<'a>,
) -> Option<&'b AstNode<'a>> {
match node.kind() {
AstKind::Function(f) if f.is_function_declaration() => return Some(node),
AstKind::Function(f) if f.is_expression() => {}
AstKind::ArrowFunctionExpression(_) => {}
_ => return None,
};
let mut current_node = node;
while let Some(parent_node) = ctx.nodes().parent_node(current_node.id()) {
match parent_node.kind() {
AstKind::VariableDeclarator(_) | AstKind::ParenthesizedExpression(_) => {
current_node = parent_node;
}
AstKind::MethodDefinition(_)
| AstKind::PropertyDefinition(_)
| AstKind::VariableDeclaration(_) => return Some(parent_node),
_ => return None,
}
}
None
}
impl Rule for NoDefaults {
fn from_configuration(value: serde_json::Value) -> Self {
value
@ -104,7 +60,11 @@ impl Rule for NoDefaults {
}
fn run<'a>(&self, node: &AstNode<'a>, ctx: &LintContext<'a>) {
let Some(jsdocs) = get_function_definition_node(node, ctx)
if !is_function_node(node) {
return;
}
let Some(jsdocs) = get_function_nearest_jsdoc_node(node, ctx)
.and_then(|node| ctx.jsdoc().get_all_by_node(node))
else {
return;

12
crates/oxc_linter/src/rules/jsdoc/require_property.rs
View file

@ -55,13 +55,16 @@ impl Rule for RequireProperty {
fn run_once(&self, ctx: &LintContext) {
let settings = &ctx.settings().jsdoc;
let resolved_property_tag_name = settings.resolve_tag_name("property");
let resolved_typedef_tag_name = settings.resolve_tag_name("typedef");
let resolved_namespace_tag_name = settings.resolve_tag_name("namespace");
for jsdoc in ctx.semantic().jsdoc().iter_all() {
let mut should_report = None;
for tag in jsdoc.tags() {
let tag_kind = tag.kind.parsed();
let tag_name = tag.kind.parsed();
if tag_kind == "typedef" || tag_kind == "namespace" {
if tag_name == resolved_typedef_tag_name || tag_name == resolved_namespace_tag_name
{
// If this is `true`:
// - This JSDoc has multiple `@typedef` or `@namespace` tags
// - And previous `@typedef` or `@namespace` tag did not have `@property` tag
@ -69,8 +72,7 @@ impl Rule for RequireProperty {
ctx.diagnostic(RequirePropertyDiagnostic(span));
}
let (type_part, _, _) = tag.type_name_comment();
let Some(type_part) = type_part else {
let (Some(type_part), _, _) = tag.type_name_comment() else {
continue;
};
@ -80,7 +82,7 @@ impl Rule for RequireProperty {
}
}
if tag_kind == resolved_property_tag_name {
if tag_name == resolved_property_tag_name {
// At least 1 `@property` tag is found
should_report = None;
}

6
crates/oxc_linter/src/rules/jsdoc/require_property_description.rs
View file

@ -47,9 +47,9 @@ impl Rule for RequirePropertyDescription {
for jsdoc in ctx.semantic().jsdoc().iter_all() {
for tag in jsdoc.tags() {
let tag_kind = tag.kind;
let tag_name = tag.kind;
if tag_kind.parsed() != resolved_property_tag_name {
if tag_name.parsed() != resolved_property_tag_name {
continue;
}
let (_, _, comment_part) = tag.type_name_comment();
@ -57,7 +57,7 @@ impl Rule for RequirePropertyDescription {
continue;
};
ctx.diagnostic(RequirePropertyDescriptionDiagnostic(tag_kind.span));
ctx.diagnostic(RequirePropertyDescriptionDiagnostic(tag_name.span));
}
}
}

6
crates/oxc_linter/src/rules/jsdoc/require_property_name.rs
View file

@ -47,9 +47,9 @@ impl Rule for RequirePropertyName {
for jsdoc in ctx.semantic().jsdoc().iter_all() {
for tag in jsdoc.tags() {
let tag_kind = tag.kind;
let tag_name = tag.kind;
if tag_kind.parsed() != resolved_property_tag_name {
if tag_name.parsed() != resolved_property_tag_name {
continue;
}
let (_, name_part, _) = tag.type_name_comment();
@ -57,7 +57,7 @@ impl Rule for RequirePropertyName {
continue;
};
ctx.diagnostic(RequirePropertyNameDiagnostic(tag_kind.span));
ctx.diagnostic(RequirePropertyNameDiagnostic(tag_name.span));
}
}
}

6
crates/oxc_linter/src/rules/jsdoc/require_property_type.rs
View file

@ -47,9 +47,9 @@ impl Rule for RequirePropertyType {
for jsdoc in ctx.semantic().jsdoc().iter_all() {
for tag in jsdoc.tags() {
let tag_kind = tag.kind;
let tag_name = tag.kind;
if tag_kind.parsed() != resolved_property_tag_name {
if tag_name.parsed() != resolved_property_tag_name {
continue;
}
let (type_part, _, _) = tag.type_name_comment();
@ -57,7 +57,7 @@ impl Rule for RequirePropertyType {
continue;
};
ctx.diagnostic(RequirePropertyTypeDiagnostic(tag_kind.span));
ctx.diagnostic(RequirePropertyTypeDiagnostic(tag_name.span));
}
}
}

38
crates/oxc_linter/src/utils/jsdoc.rs Normal file
View file

@ -0,0 +1,38 @@
use crate::{context::LintContext, AstNode};
use oxc_ast::AstKind;
/// JSDoc is often attached on the parent node of a function.
///
/// ```js
/// /** VariableDeclaration > VariableDeclarator > FunctionExpression */
/// const foo = function() {}
///
/// /** VariableDeclaration > VariableDeclarator > ArrowFunctionExpression */
/// const bar = () => {},
/// /** VariableDeclarator > ArrowFunctionExpression */
/// baz = () => {};
///
/// /** MethodDefinition > FunctionExpression */
/// class X { qux() {} }
///
/// /** PropertyDefinition > ArrowFunctionExpression */
/// class Y { qux = () => {} }
/// ```
pub fn get_function_nearest_jsdoc_node<'a, 'b>(
node: &'b AstNode<'a>,
ctx: &'b LintContext<'a>,
) -> Option<&'b AstNode<'a>> {
let mut current_node = node;
// Whether the node has attached JSDoc or not is determined by `JSDocBuilder`
while ctx.jsdoc().get_all_by_node(current_node).is_none() {
// Tie-breaker, otherwise every loop will end at `Program` node!
match current_node.kind() {
AstKind::VariableDeclaration(_)
| AstKind::MethodDefinition(_)
| AstKind::PropertyDefinition(_) => return None,
_ => current_node = ctx.nodes().parent_node(current_node.id())?,
}
}
Some(current_node)
}

5
crates/oxc_linter/src/utils/mod.rs
View file

@ -1,4 +1,5 @@
mod jest;
mod jsdoc;
mod nextjs;
mod node;
mod react;
@ -6,4 +7,6 @@ mod react_perf;
mod tree_shaking;
mod unicorn;
pub use self::{jest::*, nextjs::*, node::*, react::*, react_perf::*, tree_shaking::*, unicorn::*};
pub use self::{
jest::*, jsdoc::*, nextjs::*, node::*, react::*, react_perf::*, tree_shaking::*, unicorn::*,
};