Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 21 additions & 0 deletions src/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -247,6 +247,8 @@ impl<'a> Context<'a> {
};

let diagnostic_line = range.text_info.line_index(range.range.start);

// Ordinary directive: `// deno-lint-ignore` on the line directly above.
if diagnostic_line > 0 {
if let Some(l) =
self.line_ignore_directives.get_mut(&(diagnostic_line - 1))
Expand All @@ -257,6 +259,25 @@ impl<'a> Context<'a> {
}
}

// Block-scoped directive: `// deno-lint-ignore` before a `{ ... }` block
// suppresses diagnostics anywhere inside that block.
let mut ignored_by_block = false;
for (comment_line, directive) in self.line_ignore_directives.iter_mut() {
if let Some(block_end_line) = directive.block_end_line() {
let block_start_line = comment_line + 1;
if block_start_line <= diagnostic_line
&& diagnostic_line <= block_end_line
&& directive.check_used(&diagnostic.details.code)
{
ignored_by_block = true;
break;
}
}
}
if ignored_by_block {
continue;
}

filtered.push(diagnostic);
}

Expand Down
102 changes: 100 additions & 2 deletions src/ignore_directives.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ use deno_ast::SourceTextInfoProvider;
use deno_ast::swc::common::comments::Comment;
use deno_ast::swc::common::comments::CommentKind;
use deno_ast::view as ast_view;
use deno_ast::view::NodeTrait;
use deno_ast::RootNode;
use once_cell::sync::Lazy;
use regex::Regex;
Expand All @@ -25,6 +26,11 @@ impl DirectiveKind for File {}
pub struct IgnoreDirective<T: DirectiveKind> {
range: SourceRange,
codes: HashMap<String, CodeStatus>,
/// For a directive that immediately precedes a `{ ... }` block, the line the
/// block ends on (0-indexed, inclusive). The directive then suppresses
/// diagnostics anywhere inside the block, not just on the following line.
/// `None` for ordinary next-line directives.
block_end_line: Option<usize>,
_marker: std::marker::PhantomData<T>,
}

Expand All @@ -33,6 +39,16 @@ impl<T: DirectiveKind> IgnoreDirective<T> {
self.range
}

/// The last line (0-indexed, inclusive) covered by this directive when it is
/// block-scoped; see [`IgnoreDirective::block_end_line`] field docs.
pub fn block_end_line(&self) -> Option<usize> {
self.block_end_line
}

fn set_block_end_line(&mut self, line: usize) {
self.block_end_line = Some(line);
}

/// If the directive has no codes specified, it means all the rules should be
/// ignored.
pub fn ignore_all(&self) -> bool {
Expand Down Expand Up @@ -72,7 +88,7 @@ pub fn parse_line_ignore_directives(
ignore_diagnostic_directive: &str,
program: ast_view::Program,
) -> HashMap<usize, LineIgnoreDirective> {
program
let mut directives: HashMap<usize, LineIgnoreDirective> = program
.comment_container()
.all_comments()
.filter_map(|comment| {
Expand All @@ -89,7 +105,53 @@ pub fn parse_line_ignore_directives(
},
)
})
.collect()
.collect();

// When a directive immediately precedes a `{ ... }` block, extend its
// coverage to the whole block. Only worth walking the AST if there are
// directives to extend.
if !directives.is_empty() {
extend_block_ignore_directives(program, &mut directives);
}

directives
}

/// For every `{ ... }` block whose immediately preceding line is a
/// `deno-lint-ignore` directive, records the block's end line on that directive
/// so it suppresses diagnostics anywhere inside the block (see
/// https://github.com/denoland/deno_lint/issues/476).
///
/// Only bare/explicit blocks are covered: the directive must be a leading
/// comment of a `BlockStmt`. A directive placed before a function, class or
/// other statement attaches to that node rather than to a `BlockStmt`, so it
/// keeps the ordinary next-line behavior and does not silently swallow deeply
/// nested diagnostics.
fn extend_block_ignore_directives(
program: ast_view::Program,
directives: &mut HashMap<usize, LineIgnoreDirective>,
) {
let text_info = program.text_info();
let comments = program.comment_container();

let mut stack = vec![program.as_node()];
while let Some(node) = stack.pop() {
if let ast_view::Node::BlockStmt(block) = node {
let block_range = block.range();
let block_start_line = text_info.line_index(block_range.start);
let block_end_line = text_info.line_index(block_range.end);
for comment in comments.leading_comments(block_range.start) {
let comment_end_line = text_info.line_index(comment.range().end);
// The directive must sit on the line directly above the block's `{`.
if comment_end_line + 1 == block_start_line {
if let Some(directive) = directives.get_mut(&comment_end_line) {
directive.set_block_end_line(block_end_line);
}
}
}
}
stack.extend(node.children());
}
}

pub fn parse_file_ignore_directives(
Expand Down Expand Up @@ -177,6 +239,7 @@ fn parse_ignore_comment<T: DirectiveKind>(
return Some(IgnoreDirective::<T> {
range: comment.range(),
codes,
block_end_line: None,
_marker: std::marker::PhantomData,
});
}
Expand All @@ -190,6 +253,41 @@ mod tests {
use super::*;
use crate::test_util;

#[test]
fn test_block_scoped_directive_coverage() {
// A directive directly above a bare `{ ... }` block records the block's
// end line, so it covers the whole block.
let source_code =
"// deno-lint-ignore no-explicit-any\n{\n let a: any;\n let b: any;\n}";
test_util::parse_and_then(source_code, |program| {
let directives =
parse_line_ignore_directives("deno-lint-ignore", program);
let d = directives.get(&0).unwrap();
// The closing `}` is on line 4 (0-indexed).
assert_eq!(d.block_end_line(), Some(4));
});

// A directive above a function attaches to the function, not its body
// block, so no block coverage is recorded ("blocks only").
let source_code =
"// deno-lint-ignore no-explicit-any\nfunction foo(): any {\n let a: any;\n}";
test_util::parse_and_then(source_code, |program| {
let directives =
parse_line_ignore_directives("deno-lint-ignore", program);
let d = directives.get(&0).unwrap();
assert_eq!(d.block_end_line(), None);
});

// An ordinary next-line directive (no following block) has no coverage.
let source_code = "// deno-lint-ignore no-explicit-any\nconst a: any = 1;";
test_util::parse_and_then(source_code, |program| {
let directives =
parse_line_ignore_directives("deno-lint-ignore", program);
let d = directives.get(&0).unwrap();
assert_eq!(d.block_end_line(), None);
});
}

fn code_map(
codes: impl IntoIterator<Item = &'static str>,
) -> HashMap<String, CodeStatus> {
Expand Down
39 changes: 39 additions & 0 deletions src/linter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -303,6 +303,45 @@ mod tests {
assert_eq!(lint_with_directives(source, Some("custom-ignore")).len(), 1);
}

// A `deno-lint-ignore` directive directly above a `{ ... }` block suppresses
// diagnostics anywhere inside the block, not just on the following line.
// See https://github.com/denoland/deno_lint/issues/476.
#[test]
fn block_scoped_ignore_directive() {
// Diagnostic several lines into the block is suppressed.
let source =
"// deno-lint-ignore no-debugger\n{\n let a = 1;\n debugger;\n}";
assert!(lint_with_directives(source, None).is_empty());

// Nested blocks are covered too, but code after the block is not.
let source = "\
// deno-lint-ignore no-debugger
{
debugger;
{
debugger;
}
}
debugger;";
assert_eq!(lint_with_directives(source, None).len(), 1);

// Without a directive, a diagnostic inside the block still fires.
let source = "{\n debugger;\n}";
assert_eq!(lint_with_directives(source, None).len(), 1);

// "Blocks only": a directive before a function attaches to the function,
// not its body block, so it does not swallow deeply nested diagnostics.
// The `debugger` in the body must still be reported (the unused directive
// additionally triggers `ban-unused-ignore`, which is the desired signal).
let source =
"// deno-lint-ignore no-debugger\nfunction foo() {\n debugger;\n}";
let diagnostics = lint_with_directives(source, None);
assert!(
diagnostics.iter().any(|d| d.details.code == "no-debugger"),
"debugger in a function body must still be reported"
);
}

// With no custom directive, the default `deno-lint-ignore` still works.
#[test]
fn default_ignore_diagnostic_directive_is_respected() {
Expand Down