Skip to content
Merged
Show file tree
Hide file tree
Changes from 11 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
166 changes: 166 additions & 0 deletions crates/ruff/tests/cli/lint.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1440,6 +1440,78 @@ def function():
Ok(())
}

#[test]
fn ignore_noqa() -> Result<()> {
let fixture = CliTest::new()?;
fixture.write_file(
"ruff.toml",
r#"
[lint]
select = ["F401"]
"#,
)?;

fixture.write_file(
"noqa.py",
r#"
import os # noqa: F401

# ruff: disable[F401]
import sys
"#,
)?;

// without --ignore-noqa
assert_cmd_snapshot!(fixture
.check_command()
.args(["--config", "ruff.toml"])
.arg("noqa.py"),
@r"
success: false
exit_code: 1
----- stdout -----
noqa.py:5:8: F401 [*] `sys` imported but unused
Found 1 error.
[*] 1 fixable with the `--fix` option.

----- stderr -----
");

assert_cmd_snapshot!(fixture
.check_command()
.args(["--config", "ruff.toml"])
.arg("noqa.py")
.args(["--preview"]),
@r"
success: true
exit_code: 0
----- stdout -----
All checks passed!

----- stderr -----
");

// with --ignore-noqa --preview
assert_cmd_snapshot!(fixture
.check_command()
.args(["--config", "ruff.toml"])
.arg("noqa.py")
.args(["--ignore-noqa", "--preview"]),
@r"
success: false
exit_code: 1
----- stdout -----
noqa.py:2:8: F401 [*] `os` imported but unused
noqa.py:5:8: F401 [*] `sys` imported but unused
Found 2 errors.
[*] 2 fixable with the `--fix` option.

----- stderr -----
");

Ok(())
}

#[test]
fn add_noqa() -> Result<()> {
let fixture = CliTest::new()?;
Expand Down Expand Up @@ -1632,6 +1704,100 @@ def unused(x): # noqa: ANN001, ARG001, D103
Ok(())
}

#[test]
fn add_noqa_existing_file_level_noqa() -> Result<()> {
let fixture = CliTest::new()?;
fixture.write_file(
"ruff.toml",
r#"
[lint]
select = ["F401"]
"#,
)?;

fixture.write_file(
"noqa.py",
r#"
# ruff: noqa F401
import os
"#,
)?;

assert_cmd_snapshot!(fixture
.check_command()
.args(["--config", "ruff.toml"])
.arg("noqa.py")
.arg("--preview")
.args(["--add-noqa"])
.arg("-")
.pass_stdin(r#"

"#), @r"
success: true
exit_code: 0
----- stdout -----

----- stderr -----
");

let test_code =
fs::read_to_string(fixture.root().join("noqa.py")).expect("should read test file");
Comment on lines +1743 to +1744

@MichaReiser MichaReiser Dec 4, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We try to avoid reading fixture files in CLI tests as it makes the tests depend on each other and it can also become harder to reason about what we're testing here. Would it be possible to extract the specific case you want to test from noqa.py?

The tests here also don't need to be exhaustive. We can add more exhaustive tests to add_noqa (I think we have some integration tests, right?)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This was a copy-paste of the tests directly above and below it that exercise --add-noqa.


insta::assert_snapshot!(test_code, @r"
# ruff: noqa F401
import os
");

Ok(())
}

#[test]
fn add_noqa_existing_range_suppression() -> Result<()> {
let fixture = CliTest::new()?;
fixture.write_file(
"ruff.toml",
r#"
[lint]
select = ["F401"]
"#,
)?;

fixture.write_file(
"noqa.py",
r#"
# ruff: disable[F401]
import os
"#,
)?;

assert_cmd_snapshot!(fixture
.check_command()
.args(["--config", "ruff.toml"])
.arg("noqa.py")
.arg("--preview")
.args(["--add-noqa"])
.arg("-")
.pass_stdin(r#"

"#), @r"
success: true
exit_code: 0
----- stdout -----

----- stderr -----
");

let test_code =
fs::read_to_string(fixture.root().join("noqa.py")).expect("should read test file");

insta::assert_snapshot!(test_code, @r"
# ruff: disable[F401]
import os
");

Ok(())
}

#[test]
fn add_noqa_multiline_comment() -> Result<()> {
let fixture = CliTest::new()?;
Expand Down
56 changes: 56 additions & 0 deletions crates/ruff_linter/resources/test/fixtures/ruff/suppressions.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
def f():
# These should both be ignored by the range suppression.
# ruff: disable[E741, F841]
I = 1
# ruff: enable[E741, F841]


def f():
# These should both be ignored by the implicit range suppression.
# Should also generate an "unmatched suppression" warning.
# ruff:disable[E741,F841]
I = 1


def f():
# Neither warning is ignored, and an "unmatched suppression"
# should be generated.
I = 1
# ruff: enable[E741, F841]


def f():
# One should be ignored by the range suppression, and
# the other logged to the user.
# ruff: disable[E741]
I = 1
# ruff: enable[E741]


def f():
# Test interleaved range suppressions. The first and last
# lines should each log a different warning, while the
# middle line should be completely silenced.
# ruff: disable[E741]
l = 0
# ruff: disable[F841]
O = 1
# ruff: enable[E741]
I = 2
# ruff: enable[F841]


def f():
# Neither of these are ignored and warnings are
# logged to user
# ruff: disable[E501]
I = 1
# ruff: enable[E501]


def f():
# These should both be ignored by the range suppression,
# and an unusued noqa diagnostic should be logged.
# ruff:disable[E741,F841]
I = 1 # noqa: E741,F841
# ruff:enable[E741,F841]
14 changes: 13 additions & 1 deletion crates/ruff_linter/src/checkers/noqa.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,17 +12,20 @@ use crate::fix::edits::delete_comment;
use crate::noqa::{
Code, Directive, FileExemption, FileNoqaDirectives, NoqaDirectives, NoqaMapping,
};
use crate::preview::is_range_suppressions_enabled;
use crate::registry::Rule;
use crate::rule_redirects::get_redirect_target;
use crate::rules::pygrep_hooks;
use crate::rules::ruff;
use crate::rules::ruff::rules::{UnusedCodes, UnusedNOQA};
use crate::settings::LinterSettings;
use crate::suppression::Suppressions;
use crate::{Edit, Fix, Locator};

use super::ast::LintContext;

/// RUF100
#[expect(clippy::too_many_arguments)]
pub(crate) fn check_noqa(
context: &mut LintContext,
path: &Path,
Expand All @@ -31,6 +34,7 @@ pub(crate) fn check_noqa(
noqa_line_for: &NoqaMapping,
analyze_directives: bool,
settings: &LinterSettings,
suppressions: &Suppressions,
) -> Vec<usize> {
// Identify any codes that are globally exempted (within the current file).
let file_noqa_directives =
Expand All @@ -40,7 +44,7 @@ pub(crate) fn check_noqa(
let mut noqa_directives =
NoqaDirectives::from_commented_ranges(comment_ranges, &settings.external, path, locator);

if file_noqa_directives.is_empty() && noqa_directives.is_empty() {
if file_noqa_directives.is_empty() && noqa_directives.is_empty() && suppressions.is_empty() {
return Vec::new();
}

Expand All @@ -60,11 +64,19 @@ pub(crate) fn check_noqa(
continue;
}

// Apply file-level suppressions first
if exemption.contains_secondary_code(code) {
ignored_diagnostics.push(index);
continue;
}

// Apply ranged suppressions next
if is_range_suppressions_enabled(settings) && suppressions.check_diagnostic(diagnostic) {
ignored_diagnostics.push(index);
continue;
}

// Apply end-of-line noqa suppressions last
let noqa_offsets = diagnostic
.parent()
.into_iter()
Expand Down
15 changes: 15 additions & 0 deletions crates/ruff_linter/src/linter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,12 +26,14 @@ use crate::doc_lines::{doc_lines_from_ast, doc_lines_from_tokens};
use crate::fix::{FixResult, fix_file};
use crate::noqa::add_noqa;
use crate::package::PackageRoot;
use crate::preview::is_range_suppressions_enabled;
use crate::registry::Rule;
#[cfg(any(feature = "test-rules", test))]
use crate::rules::ruff::rules::test_rules::{self, TEST_RULES, TestRule};
use crate::settings::types::UnsafeFixes;
use crate::settings::{LinterSettings, TargetVersion, flags};
use crate::source_kind::SourceKind;
use crate::suppression::Suppressions;
use crate::{Locator, directives, fs};

pub(crate) mod float;
Expand Down Expand Up @@ -331,6 +333,11 @@ pub fn check_path(
.iter_enabled_rules()
.any(|rule_code| rule_code.lint_source().is_noqa())
{
let suppressions = if is_range_suppressions_enabled(settings) {
Suppressions::from_tokens(locator.contents(), tokens)
} else {
Suppressions::default()
};
let ignored = check_noqa(
&mut context,
path,
Expand All @@ -339,6 +346,7 @@ pub fn check_path(
&directives.noqa_line_for,
parsed.has_valid_syntax(),
settings,
&suppressions,
);
if noqa.is_enabled() {
for index in ignored.iter().rev() {
Expand Down Expand Up @@ -416,6 +424,12 @@ pub fn add_noqa_to_path(
target_version,
);

let suppressions = if is_range_suppressions_enabled(settings) {
Suppressions::from_tokens(locator.contents(), parsed.tokens())
} else {
Suppressions::default()
};

Comment thread
amyreese marked this conversation as resolved.
Outdated
// Add any missing `# noqa` pragmas.
// TODO(dhruvmanila): Add support for Jupyter Notebooks
add_noqa(
Expand All @@ -427,6 +441,7 @@ pub fn add_noqa_to_path(
&directives.noqa_line_for,
stylist.line_ending(),
reason,
&suppressions,
)
}

Expand Down
Loading