Remove E999 as a rule, disallow any disablement methods for syntax …

…error (#11901)

## Summary

This PR updates the way syntax errors are handled throughout the linter.

The main change is that it's now not considered as a rule which involves
the following changes:
* Update `Message` to be an enum with two variants - one for diagnostic
message and the other for syntax error message
* Provide methods on the new message enum to query information required
by downstream usages

This means that the syntax errors cannot be hidden / disabled via any
disablement methods. These are:
1. Configuration via `select`, `ignore`, `per-file-ignores`, and their
`extend-*` variants
	```console
$ cargo run -- check ~/playground/ruff/src/lsp.py --extend-select=E999
--no-preview --no-cache
	    Finished `dev` profile [unoptimized + debuginfo] target(s) in 0.10s
Running `target/debug/ruff check /Users/dhruv/playground/ruff/src/lsp.py
--extend-select=E999 --no-preview --no-cache`
warning: Rule `E999` is deprecated and will be removed in a future
release. Syntax errors will always be shown regardless of whether this
rule is selected or not.
/Users/dhruv/playground/ruff/src/lsp.py:1:8: F401 [*] `abc` imported but
unused
	  |
	1 | import abc
	  |        ^^^ F401
	2 | from pathlib import Path
	3 | import os
	  |
	  = help: Remove unused import: `abc`
	```
3. Command-line flags via `--select`, `--ignore`, `--per-file-ignores`,
and their `--extend-*` variants
	```console
$ cargo run -- check ~/playground/ruff/src/lsp.py --no-cache
--config=~/playground/ruff/pyproject.toml
	    Finished `dev` profile [unoptimized + debuginfo] target(s) in 0.11s
Running `target/debug/ruff check /Users/dhruv/playground/ruff/src/lsp.py
--no-cache --config=/Users/dhruv/playground/ruff/pyproject.toml`
warning: Rule `E999` is deprecated and will be removed in a future
release. Syntax errors will always be shown regardless of whether this
rule is selected or not.
/Users/dhruv/playground/ruff/src/lsp.py:1:8: F401 [*] `abc` imported but
unused
	  |
	1 | import abc
	  |        ^^^ F401
	2 | from pathlib import Path
	3 | import os
	  |
	  = help: Remove unused import: `abc`
	```

This also means that the **output format** needs to be updated:
1. The `code`, `noqa_row`, `url` fields in the JSON output is optional
(`null` for syntax errors)
2. Other formats are changed accordingly
For each format, a new test case specific to syntax errors have been
added. Please refer to the snapshot output for the exact format for
syntax error message.

The output of the `--statistics` flag will have a blank entry for syntax
errors:
```
315     F821    [ ] undefined-name
119             [ ] syntax-error
103     F811    [ ] redefined-while-unused
```

The **language server** is updated to consider the syntax errors by
convert them into LSP diagnostic format separately.

### Preview

There are no quick fixes provided to disable syntax errors. This will
automatically work for `ruff-lsp` because the `noqa_row` field will be
`null` in that case.
<img width="772" alt="Screenshot 2024-06-26 at 14 57 08"
src="https://github.com/astral-sh/ruff/assets/67177269/aaac827e-4777-4ac8-8c68-eaf9f2c36774">

Even with `noqa` comment, the syntax error is displayed:
<img width="763" alt="Screenshot 2024-06-26 at 14 59 51"
src="https://github.com/astral-sh/ruff/assets/67177269/ba1afb68-7eaf-4b44-91af-6d93246475e2">

Rule documentation page:
<img width="1371" alt="Screenshot 2024-06-26 at 16 48 07"
src="https://github.com/astral-sh/ruff/assets/67177269/524f01df-d91f-4ac0-86cc-40e76b318b24">


## Test Plan

- [x] Disablement methods via config shows a warning
	- [x] `select`, `extend-select`
	- [ ] ~`ignore`~ _doesn't show any message_
- [ ] ~`per-file-ignores`, `extend-per-file-ignores`~ _doesn't show any
message_
- [x] Disablement methods via command-line flag shows a warning
	- [x] `--select`, `--extend-select`
	- [ ] ~`--ignore`~ _doesn't show any message_
- [ ] ~`--per-file-ignores`, `--extend-per-file-ignores`~ _doesn't show
any message_
- [x] File with syntax errors should exit with code 1
- [x] Language server
	- [x] Should show diagnostics for syntax errors
	- [x] Should not recommend a quick fix edit for adding `noqa` comment
	- [x] Same for `ruff-lsp`

resolves: #8447

crates/ruff/src/cache.rs

@@ -19,7 +19,7 @@ use tempfile::NamedTempFile;
 
 use ruff_cache::{CacheKey, CacheKeyHasher};
 use ruff_diagnostics::{DiagnosticKind, Fix};
-use ruff_linter::message::Message;
+use ruff_linter::message::{DiagnosticMessage, Message};
 use ruff_linter::{warn_user, VERSION};
 use ruff_macros::CacheKey;
 use ruff_notebook::NotebookIndex;
@@ -333,12 +333,14 @@ impl FileCache {
                 let file = SourceFileBuilder::new(path.to_string_lossy(), &*lint.source).finish();
                 lint.messages
                     .iter()
-                    .map(|msg| Message {
-                        kind: msg.kind.clone(),
-                        range: msg.range,
-                        fix: msg.fix.clone(),
-                        file: file.clone(),
-                        noqa_offset: msg.noqa_offset,
+                    .map(|msg| {
+                        Message::Diagnostic(DiagnosticMessage {
+                            kind: msg.kind.clone(),
+                            range: msg.range,
+                            fix: msg.fix.clone(),
+                            file: file.clone(),
+                            noqa_offset: msg.noqa_offset,
+                        })
                     })
                     .collect()
             };
@@ -412,18 +414,19 @@ impl LintCacheData {
         notebook_index: Option<NotebookIndex>,
     ) -> Self {
         let source = if let Some(msg) = messages.first() {
-            msg.file.source_text().to_owned()
+            msg.source_file().source_text().to_owned()
         } else {
             String::new() // No messages, no need to keep the source!
         };
 
         let messages = messages
             .iter()
+            .filter_map(|message| message.as_diagnostic_message())
             .map(|msg| {
                 // Make sure that all message use the same source file.
                 assert_eq!(
-                    msg.file,
-                    messages.first().unwrap().file,
+                    &msg.file,
+                    messages.first().unwrap().source_file(),
                     "message uses a different source file"
                 );
                 CacheMessage {
@@ -571,6 +574,7 @@ mod tests {
     use test_case::test_case;
 
     use ruff_cache::CACHE_DIR_NAME;
+    use ruff_linter::message::Message;
     use ruff_linter::settings::flags;
     use ruff_linter::settings::types::UnsafeFixes;
     use ruff_python_ast::PySourceType;
@@ -633,11 +637,7 @@ mod tests {
                     UnsafeFixes::Enabled,
                 )
                 .unwrap();
-                if diagnostics
-                    .messages
-                    .iter()
-                    .any(|m| m.kind.name == "SyntaxError")
-                {
+                if diagnostics.messages.iter().any(Message::is_syntax_error) {
                     parse_errors.push(path.clone());
                 }
                 paths.push(path);

crates/ruff/src/diagnostics.rs

@@ -10,18 +10,18 @@ use std::path::Path;
 use anyhow::{Context, Result};
 use colored::Colorize;
 use log::{debug, error, warn};
+use ruff_linter::codes::Rule;
 use rustc_hash::FxHashMap;
 
 use ruff_diagnostics::Diagnostic;
 use ruff_linter::linter::{lint_fix, lint_only, FixTable, FixerResult, LinterResult, ParseSource};
 use ruff_linter::logging::DisplayParseError;
-use ruff_linter::message::Message;
+use ruff_linter::message::{Message, SyntaxErrorMessage};
 use ruff_linter::pyproject_toml::lint_pyproject_toml;
-use ruff_linter::registry::AsRule;
 use ruff_linter::settings::types::UnsafeFixes;
 use ruff_linter::settings::{flags, LinterSettings};
 use ruff_linter::source_kind::{SourceError, SourceKind};
-use ruff_linter::{fs, IOError, SyntaxError};
+use ruff_linter::{fs, IOError};
 use ruff_notebook::{Notebook, NotebookError, NotebookIndex};
 use ruff_python_ast::{PySourceType, SourceType, TomlSourceType};
 use ruff_source_file::SourceFileBuilder;
@@ -55,57 +55,61 @@ impl Diagnostics {
         path: Option<&Path>,
         settings: &LinterSettings,
     ) -> Self {
-        let diagnostic = match err {
+        match err {
             // IO errors.
             SourceError::Io(_)
             | SourceError::Notebook(NotebookError::Io(_) | NotebookError::Json(_)) => {
-                Diagnostic::new(
-                    IOError {
-                        message: err.to_string(),
-                    },
-                    TextRange::default(),
-                )
+                if settings.rules.enabled(Rule::IOError) {
+                    let name = path.map_or_else(|| "-".into(), Path::to_string_lossy);
+                    let source_file = SourceFileBuilder::new(name, "").finish();
+                    Self::new(
+                        vec![Message::from_diagnostic(
+                            Diagnostic::new(
+                                IOError {
+                                    message: err.to_string(),
+                                },
+                                TextRange::default(),
+                            ),
+                            source_file,
+                            TextSize::default(),
+                        )],
+                        FxHashMap::default(),
+                    )
+                } else {
+                    match path {
+                        Some(path) => {
+                            warn!(
+                                "{}{}{} {err}",
+                                "Failed to lint ".bold(),
+                                fs::relativize_path(path).bold(),
+                                ":".bold()
+                            );
+                        }
+                        None => {
+                            warn!("{}{} {err}", "Failed to lint".bold(), ":".bold());
+                        }
+                    }
+
+                    Self::default()
+                }
             }
             // Syntax errors.
             SourceError::Notebook(
                 NotebookError::InvalidJson(_)
                 | NotebookError::InvalidSchema(_)
                 | NotebookError::InvalidFormat(_),
-            ) => Diagnostic::new(
-                SyntaxError {
-                    message: err.to_string(),
-                },
-                TextRange::default(),
-            ),
-        };
-
-        if settings.rules.enabled(diagnostic.kind.rule()) {
-            let name = path.map_or_else(|| "-".into(), Path::to_string_lossy);
-            let dummy = SourceFileBuilder::new(name, "").finish();
-            Self::new(
-                vec![Message::from_diagnostic(
-                    diagnostic,
-                    dummy,
-                    TextSize::default(),
-                )],
-                FxHashMap::default(),
-            )
-        } else {
-            match path {
-                Some(path) => {
-                    warn!(
-                        "{}{}{} {err}",
-                        "Failed to lint ".bold(),
-                        fs::relativize_path(path).bold(),
-                        ":".bold()
-                    );
-                }
-                None => {
-                    warn!("{}{} {err}", "Failed to lint".bold(), ":".bold());
-                }
+            ) => {
+                let name = path.map_or_else(|| "-".into(), Path::to_string_lossy);
+                let dummy = SourceFileBuilder::new(name, "").finish();
+                Self::new(
+                    vec![Message::SyntaxError(SyntaxErrorMessage {
+                        message: err.to_string(),
+                        range: TextRange::default(),
+                        file: dummy,
+                    })],
+                    FxHashMap::default(),
+                )
             }
-
-            Self::default()
         }
     }
 }

crates/ruff/src/printer.rs

@@ -13,11 +13,11 @@ use ruff_linter::fs::relativize_path;
 use ruff_linter::logging::LogLevel;
 use ruff_linter::message::{
     AzureEmitter, Emitter, EmitterContext, GithubEmitter, GitlabEmitter, GroupedEmitter,
-    JsonEmitter, JsonLinesEmitter, JunitEmitter, PylintEmitter, RdjsonEmitter, SarifEmitter,
-    TextEmitter,
+    JsonEmitter, JsonLinesEmitter, JunitEmitter, Message, MessageKind, PylintEmitter,
+    RdjsonEmitter, SarifEmitter, TextEmitter,
 };
 use ruff_linter::notify_user;
-use ruff_linter::registry::{AsRule, Rule};
+use ruff_linter::registry::Rule;
 use ruff_linter::settings::flags::{self};
 use ruff_linter::settings::types::{OutputFormat, UnsafeFixes};
 
@@ -37,12 +37,13 @@ bitflags! {
 
 #[derive(Serialize)]
 struct ExpandedStatistics {
-    code: SerializeRuleAsCode,
-    name: SerializeRuleAsTitle,
+    code: Option<SerializeRuleAsCode>,
+    name: SerializeMessageKindAsTitle,
     count: usize,
     fixable: bool,
 }
 
+#[derive(Copy, Clone)]
 struct SerializeRuleAsCode(Rule);
 
 impl Serialize for SerializeRuleAsCode {
@@ -66,26 +67,26 @@ impl From<Rule> for SerializeRuleAsCode {
     }
 }
 
-struct SerializeRuleAsTitle(Rule);
+struct SerializeMessageKindAsTitle(MessageKind);
 
-impl Serialize for SerializeRuleAsTitle {
+impl Serialize for SerializeMessageKindAsTitle {
     fn serialize<S>(&self, serializer: S) -> std::result::Result<S::Ok, S::Error>
     where
         S: serde::Serializer,
     {
-        serializer.serialize_str(self.0.as_ref())
+        serializer.serialize_str(self.0.as_str())
     }
 }
 
-impl Display for SerializeRuleAsTitle {
+impl Display for SerializeMessageKindAsTitle {
     fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
-        write!(f, "{}", self.0.as_ref())
+        f.write_str(self.0.as_str())
     }
 }
 
-impl From<Rule> for SerializeRuleAsTitle {
-    fn from(rule: Rule) -> Self {
-        Self(rule)
+impl From<MessageKind> for SerializeMessageKindAsTitle {
+    fn from(kind: MessageKind) -> Self {
+        Self(kind)
     }
 }
 
@@ -341,24 +342,23 @@ impl Printer {
         let statistics: Vec<ExpandedStatistics> = diagnostics
             .messages
             .iter()
-            .map(|message| (message.kind.rule(), message.fix.is_some()))
-            .sorted()
-            .fold(vec![], |mut acc, (rule, fixable)| {
-                if let Some((prev_rule, _, count)) = acc.last_mut() {
-                    if *prev_rule == rule {
+            .sorted_by_key(|message| (message.rule(), message.fixable()))
+            .fold(vec![], |mut acc: Vec<(&Message, usize)>, message| {
+                if let Some((prev_message, count)) = acc.last_mut() {
+                    if prev_message.rule() == message.rule() {
                         *count += 1;
                         return acc;
                     }
                 }
-                acc.push((rule, fixable, 1));
+                acc.push((message, 1));
                 acc
             })
             .iter()
-            .map(|(rule, fixable, count)| ExpandedStatistics {
-                code: (*rule).into(),
-                name: (*rule).into(),
-                count: *count,
-                fixable: *fixable,
+            .map(|&(message, count)| ExpandedStatistics {
+                code: message.rule().map(std::convert::Into::into),
+                name: message.kind().into(),
+                count,
+                fixable: message.fixable(),
             })
             .sorted_by_key(|statistic| Reverse(statistic.count))
             .collect();
@@ -381,7 +381,12 @@ impl Printer {
                 );
                 let code_width = statistics
                     .iter()
-                    .map(|statistic| statistic.code.to_string().len())
+                    .map(|statistic| {
+                        statistic
+                            .code
+                            .map_or_else(String::new, |rule| rule.to_string())
+                            .len()
+                    })
                     .max()
                     .unwrap();
                 let any_fixable = statistics.iter().any(|statistic| statistic.fixable);
@@ -395,7 +400,11 @@ impl Printer {
                         writer,
                         "{:>count_width$}\t{:<code_width$}\t{}{}",
                         statistic.count.to_string().bold(),
-                        statistic.code.to_string().red().bold(),
+                        statistic
+                            .code
+                            .map_or_else(String::new, |rule| rule.to_string())
+                            .red()
+                            .bold(),
                         if any_fixable {
                             if statistic.fixable {
                                 &fixable
@@ -545,7 +554,7 @@ impl FixableStatistics {
         let mut unapplicable_unsafe = 0;
 
         for message in &diagnostics.messages {
-            if let Some(fix) = &message.fix {
+            if let Some(fix) = message.fix() {
                 if fix.applies(unsafe_fixes.required_applicability()) {
                     applicable += 1;
                 } else {