refactor(rome_rowan): Remove `AstNode` constraint from `BatchMutation`
MODIFY
crates/rome_analyze/src/rule.rs
crates/rome_analyze/src/rule.rs
use crate::categories::{ActionCategory, RuleCategory};
use crate::context::RuleContext;
use crate::registry::{RuleLanguage, RuleSuppressions};
-use crate::{
- AnalysisFilter, AnalyzerDiagnostic, LanguageRoot, Phase, Phases, Queryable, RuleRegistry,
-};
+use crate::{AnalysisFilter, AnalyzerDiagnostic, Phase, Phases, Queryable, RuleRegistry};
use rome_console::fmt::Display;
use rome_console::{markup, MarkupBuf};
use rome_diagnostics::{file::FileId, Applicability, Severity};
@@ -447,5 +445,5 @@ pub struct RuleAction<L: Language> {
pub category: ActionCategory,
pub applicability: Applicability,
pub message: MarkupBuf,
- pub mutation: BatchMutation<L, LanguageRoot<L>>,
+ pub mutation: BatchMutation<L>,
}

MODIFY
crates/rome_analyze/src/signals.rs
crates/rome_analyze/src/signals.rs
use crate::{
categories::ActionCategory,
context::RuleContext,
- registry::{LanguageRoot, RuleLanguage, RuleRoot},
+ registry::{RuleLanguage, RuleRoot},
rule::Rule,
AnalyzerDiagnostic, Queryable, RuleGroup, ServiceBag,
};
@@ -61,7 +61,7 @@ pub struct AnalyzerAction<L: Language> {
pub category: ActionCategory,
pub applicability: Applicability,
pub message: MarkupBuf,
- pub mutation: BatchMutation<L, LanguageRoot<L>>,
+ pub mutation: BatchMutation<L>,
}
impl<L> From<AnalyzerAction<L>> for CodeSuggestion

MODIFY
crates/rome_js_analyze/src/lib.rs
crates/rome_js_analyze/src/lib.rs
@@ -9,6 +9,7 @@ use rome_js_syntax::{
suppression::{parse_suppression_comment, SuppressionCategory},
JsLanguage,
};
+use std::error::Error;
mod analyzers;
mod assists;
@@ -155,3 +156,25 @@ mod tests {
);
}
}
+
+/// Series of errors encountered when running rules on a file
+#[derive(Debug, PartialEq, Copy, Clone)]
+pub enum RuleError {
+ /// The rule with the specified name replaced the root of the file with a node that is not a valid root for that language.
+ ReplacedRootWithNonRootError { rule_name: &'static str },
+}
+
+impl std::fmt::Display for RuleError {
+ fn fmt(&self, fmt: &mut std::fmt::Formatter) -> std::fmt::Result {
+ match self {
+ RuleError::ReplacedRootWithNonRootError { rule_name } => {
+ std::write!(
+ fmt,
+ "the rule '{rule_name}' replaced the root of the file with a non-root node."
+ )
+ }
+ }
+ }
+}
+
+impl Error for RuleError {}

MODIFY
crates/rome_js_analyze/src/utils.rs
crates/rome_js_analyze/src/utils.rs
use rome_js_factory::make;
-use rome_js_syntax::{JsAnyRoot, JsAnyStatement, JsLanguage, JsModuleItemList, JsStatementList, T};
+use rome_js_syntax::{JsAnyStatement, JsLanguage, JsModuleItemList, JsStatementList, T};
use rome_rowan::{AstNode, BatchMutation};
use std::borrow::Cow;
@@ -123,10 +123,7 @@ pub fn to_camel_case(input: &str) -> Cow<str> {
/// Utility function to remove a statement node from a syntax tree, by either
/// removing the node from its parent if said parent is a statement list or
/// module item list, or by replacing the statement node with an empty statement
-pub(crate) fn remove_statement<N>(
- mutation: &mut BatchMutation<JsLanguage, JsAnyRoot>,
- node: &N,
-) -> Option<()>
+pub(crate) fn remove_statement<N>(mutation: &mut BatchMutation<JsLanguage>, node: &N) -> Option<()>
where
N: AstNode<Language = JsLanguage> + Into<JsAnyStatement>,
{

MODIFY
crates/rome_js_analyze/src/utils/batch.rs
crates/rome_js_analyze/src/utils/batch.rs
use rome_js_syntax::{
- JsAnyFormalParameter, JsAnyParameter, JsAnyRoot, JsFormalParameter, JsLanguage,
- JsParameterList, JsVariableDeclaration, JsVariableDeclarator, JsVariableDeclaratorList,
- JsVariableStatement,
+ JsAnyFormalParameter, JsAnyParameter, JsFormalParameter, JsLanguage, JsParameterList,
+ JsVariableDeclaration, JsVariableDeclarator, JsVariableDeclaratorList, JsVariableStatement,
};
use rome_rowan::{AstNode, AstSeparatedList, BatchMutation};
@@ -16,7 +15,7 @@ pub trait JsBatchMutation {
fn remove_js_formal_parameter(&mut self, parameter: &JsFormalParameter);
}
-impl JsBatchMutation for BatchMutation<JsLanguage, JsAnyRoot> {
+impl JsBatchMutation for BatchMutation<JsLanguage> {
fn remove_js_variable_declarator(&mut self, declarator: &JsVariableDeclarator) {
declarator
.parent::<JsVariableDeclaratorList>()

MODIFY
crates/rome_js_analyze/tests/spec_tests.rs
crates/rome_js_analyze/tests/spec_tests.rs
@@ -15,7 +15,6 @@ use rome_js_parser::{
test_utils::{assert_errors_are_absent, has_unknown_nodes_or_empty_slots},
};
use rome_js_syntax::{JsLanguage, SourceType};
-use rome_rowan::AstNode;
use rome_text_edit::apply_indels;
tests_macros::gen_tests! {"tests/specs/**/*.{cjs,js,jsx,tsx,ts}", crate::run_test, "module"}
@@ -155,7 +154,7 @@ fn check_code_action(
// returns the same code as printing the modified syntax tree
assert_eq!(new_tree.to_string(), output);
- if has_unknown_nodes_or_empty_slots(new_tree.syntax()) {
+ if has_unknown_nodes_or_empty_slots(&new_tree) {
panic!("modified tree has unknown nodes or empty slots:\n{new_tree:#?}")
}

MODIFY
crates/rome_rowan/benches/mutation.rs
crates/rome_rowan/benches/mutation.rs
@@ -63,7 +63,7 @@ fn mutation_batch() -> usize {
batch.replace_node(a, b);
let root = batch.commit();
- root.syntax().descendants().count()
+ root.descendants().count()
}
iai::main!(mutation_replace_node, mutation_batch);

MODIFY
crates/rome_rowan/src/ast/batch.rs
crates/rome_rowan/src/ast/batch.rs
use rome_text_edit::Indel;
use text_size::TextRange;
-use crate::{
- AstNode, Language, SyntaxElement, SyntaxKind, SyntaxNode, SyntaxNodeCast, SyntaxSlot,
- SyntaxToken,
-};
+use crate::{AstNode, Language, SyntaxElement, SyntaxKind, SyntaxNode, SyntaxSlot, SyntaxToken};
use std::{
- any::type_name,
cmp,
collections::BinaryHeap,
iter::{empty, once},
@@ -17,7 +13,7 @@ where
L: Language,
{
#[must_use = "This method consumes the node and return the BatchMutation api that returns the new SynytaxNode on commit"]
- fn begin(self) -> BatchMutation<L, Self>;
+ fn begin(self) -> BatchMutation<L>;
}
impl<L, T> BatchMutationExt<L> for T
@@ -26,11 +22,8 @@ where
T: AstNode<Language = L>,
{
#[must_use = "This method consumes the node and return the BatchMutation api that returns the new SynytaxNode on commit"]
- fn begin(self) -> BatchMutation<L, Self> {
- BatchMutation {
- root: self,
- changes: BinaryHeap::new(),
- }
+ fn begin(self) -> BatchMutation<L> {
+ BatchMutation::new(self.into_syntax())
}
}
@@ -88,20 +81,25 @@ impl<L: Language> Ord for CommitChange<L> {
}
#[derive(Debug, Clone)]
-pub struct BatchMutation<L, N>
+pub struct BatchMutation<L>
where
L: Language,
- N: AstNode<Language = L>,
{
- root: N,
+ root: SyntaxNode<L>,
changes: BinaryHeap<CommitChange<L>>,
}
-impl<L, N> BatchMutation<L, N>
+impl<L> BatchMutation<L>
where
L: Language,
- N: AstNode<Language = L>,
{
+ pub fn new(root: SyntaxNode<L>) -> Self {
+ Self {
+ root,
+ changes: BinaryHeap::new(),
+ }
+ }
+
/// Push a change to replace the "prev_node" with "next_node".
/// Trivia from "prev_node" is automatically copied to "next_node".
///
@@ -264,7 +262,7 @@ where
/// [None] if the mutation is empty
pub fn as_text_edits(&self) -> Option<(TextRange, Vec<Indel>)> {
let iter = self.changes.iter().filter_map(|change| {
- let parent = change.parent.as_ref().unwrap_or_else(|| self.root.syntax());
+ let parent = change.parent.as_ref().unwrap_or(&self.root);
let delete = match parent.slots().nth(change.new_node_slot) {
Some(SyntaxSlot::Node(node)) => node.text_range(),
Some(SyntaxSlot::Token(token)) => token.text_range(),
@@ -310,7 +308,7 @@ where
/// To address this case at step 3, when we pop a new change to apply it, we actually aggregate all changes to the current
/// parent together. This is done by the heap because we also sort by node and it's range.
///
- pub fn commit(self) -> N {
+ pub fn commit(self) -> SyntaxNode<L> {
let BatchMutation { root, mut changes } = self;
// Fill the heap with the requested changes
@@ -376,16 +374,9 @@ where
.new_node
.expect("new_node")
.into_node()
- .expect("into_node");
- let kind = root.kind();
- match root.cast() {
- Some(root) => return root,
- None => panic!(
- "failed to cast root node with kind {:?} into {}",
- kind,
- type_name::<N>()
- ),
- };
+ .expect("expected root to be a node and not a token");
+
+ return root;
}
}
@@ -467,7 +458,7 @@ pub mod tests {
let batch = before.begin();
let after = batch.commit();
- assert_eq!(before_debug, format!("{:#?}", after.syntax()));
+ assert_eq!(before_debug, format!("{:#?}", after));
}
#[test]
@@ -482,7 +473,7 @@ pub mod tests {
batch.replace_node(a, b);
let root = batch.commit();
- assert_eq!(expected_debug, format!("{:#?}", root.syntax()));
+ assert_eq!(expected_debug, format!("{:#?}", root));
}
#[test]
@@ -500,6 +491,6 @@ pub mod tests {
batch.replace_node(b, d);
let after = batch.commit();
- assert_eq!(expected_debug, format!("{:#?}", after.syntax()));
+ assert_eq!(expected_debug, format!("{:#?}", after));
}
}

MODIFY
crates/rome_service/src/file_handlers/javascript.rs
crates/rome_service/src/file_handlers/javascript.rs
@@ -2,8 +2,8 @@ use rome_analyze::{AnalysisFilter, ControlFlow, Never, RuleCategories, RuleFilte
use rome_diagnostics::{Applicability, CodeSuggestion, Diagnostic};
use rome_formatter::{FormatError, Printed};
use rome_fs::RomePath;
-use rome_js_analyze::analyze;
use rome_js_analyze::utils::rename::RenameError;
+use rome_js_analyze::{analyze, RuleError};
use rome_js_formatter::context::QuoteStyle;
use rome_js_formatter::{context::JsFormatContext, format_node};
use rome_js_parser::Parse;
@@ -262,7 +262,16 @@ fn fix_all(params: FixAllParams) -> Result<FixFileResult, RomeError> {
match action {
Some(action) => {
if let Some((range, _)) = action.mutation.as_text_edits() {
- tree = action.mutation.commit();
+ tree = match JsAnyRoot::cast(action.mutation.commit()) {
+ Some(tree) => tree,
+ None => {
+ return Err(RomeError::RuleError(
+ RuleError::ReplacedRootWithNonRootError {
+ rule_name: action.rule_name,
+ },
+ ))
+ }
+ };
actions.push(FixAction {
rule_name: Cow::Borrowed(action.rule_name),
range,

MODIFY
crates/rome_service/src/lib.rs
crates/rome_service/src/lib.rs
@@ -2,6 +2,7 @@ use rome_console::{Console, EnvConsole};
use rome_formatter::FormatError;
use rome_fs::{FileSystem, OsFileSystem, RomePath};
use rome_js_analyze::utils::rename::RenameError;
+use rome_js_analyze::RuleError;
use std::error::Error;
use std::fmt::{Debug, Display, Formatter};
use std::ops::{Deref, DerefMut};
@@ -42,6 +43,8 @@ pub enum RomeError {
FormatError(FormatError),
/// The file could not be formatted since it has syntax errors and `format_with_errors` is disabled
FormatWithErrorsDisabled,
+ /// The file could not be analyzed because a rule caused an error.
+ RuleError(RuleError),
/// Thrown when Rome can't read a generic directory
CantReadDirectory(PathBuf),
/// Thrown when Rome can't read a generic file
@@ -54,17 +57,7 @@ pub enum RomeError {
impl Debug for RomeError {
fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
- match self {
- RomeError::NotFound => std::fmt::Display::fmt(self, f),
- RomeError::SourceFileNotSupported(_) => std::fmt::Display::fmt(self, f),
- RomeError::FormatError(_) => std::fmt::Display::fmt(self, f),
- RomeError::FormatWithErrorsDisabled => std::fmt::Display::fmt(self, f),
- RomeError::CantReadDirectory(_) => std::fmt::Display::fmt(self, f),
- RomeError::CantReadFile(_) => std::fmt::Display::fmt(self, f),
- RomeError::Configuration(_) => std::fmt::Display::fmt(self, f),
- RomeError::DirtyWorkspace => std::fmt::Display::fmt(self, f),
- RomeError::RenameError(_) => std::fmt::Display::fmt(self, f),
- }
+ std::fmt::Display::fmt(self, f)
}
}
@@ -129,6 +122,12 @@ impl Display for RomeError {
)
}
},
+ RomeError::RuleError(cause) => {
+ write!(
+ f,
+ "the linter encountered an error while analyzing the file: {cause}",
+ )
+ }
}
}
}

refactor(rome_rowan): Remove `AstNode` constraint from `BatchMutation` (#3026)