diff --git a/crates/docblock/src/error.rs b/crates/docblock/src/error.rs index be91688..4db2d8c 100644 --- a/crates/docblock/src/error.rs +++ b/crates/docblock/src/error.rs @@ -51,49 +51,49 @@ impl std::fmt::Display for ParseError { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { match self { ParseError::InvalidTrivia(_) => { - write!(f, "invalid trivia") + write!(f, "Invalid docblock comment.") } ParseError::UnclosedInlineTag(_) => { - write!(f, "unclosed inline tag") + write!(f, "Unclosed inline tag.") } ParseError::UnclosedInlineCode(_) => { - write!(f, "unclosed inline code") + write!(f, "Unclosed inline code.") } ParseError::UnclosedCodeBlock(_) => { - write!(f, "unclosed code block") + write!(f, "Unclosed code block.") } ParseError::InvalidTagName(_) => { - write!(f, "invalid tag name") + write!(f, "Invalid tag name.") } ParseError::InvalidAnnotationName(_) => { - write!(f, "invalid annotation name") + write!(f, "Invalid annotation name.") } ParseError::UnclosedAnnotationArguments(_) => { - write!(f, "unclosed annotation arguments") + write!(f, "Unclosed annotation arguments.") } ParseError::MalformedCodeBlock(_) => { - write!(f, "malformed code block") + write!(f, "Malformed code block.") } ParseError::InvalidComment(_) => { - write!(f, "invalid comment") + write!(f, "Invalid comment.") } ParseError::InconsistentIndentation(_, _, _) => { - write!(f, "inconsistent indentation") + write!(f, "Inconsistent indentation.") } ParseError::MissingAsterisk(_) => { - write!(f, "missing `*` in a docblock comment") + write!(f, "Missing `*` in a docblock comment.") } ParseError::MissingWhitespaceAfterAsterisk(_) => { - write!(f, "missing whitespace after `*`") + write!(f, "Missing whitespace after `*`.") } ParseError::MissingWhitespaceAfterOpeningAsterisk(_) => { - write!(f, "missing whitespace after `/*` in a single-line docblock") + write!(f, "Missing whitespace after `/*` in a single-line docblock.") } ParseError::MissingWhitespaceBeforeClosingAsterisk(_) => { - write!(f, "missing whitespace before `*/` in a single-line docblock") + write!(f, "Missing whitespace before `*/` in a single-line docblock.") } ParseError::ExpectedLine(_) => { - write!(f, "missing expected line") + write!(f, "Missing expected line.") } } } @@ -102,41 +102,41 @@ impl std::fmt::Display for ParseError { impl ParseError { pub fn note(&self) -> &'static str { match self { - ParseError::InvalidTrivia(_) => "the comment is not recognized as a docblock. It should start with '/**' and end with '*/'.", - ParseError::UnclosedInlineTag(_) => "the inline tag is missing a closing '}'.", - ParseError::UnclosedInlineCode(_) => "inline code is missing a closing backtick '`'.", - ParseError::UnclosedCodeBlock(_) => "the code block is missing a closing delimiter '```'.", - ParseError::InvalidTagName(_) => "the tag name contains invalid characters.", - ParseError::InvalidAnnotationName(_) => "the annotation name is invalid. It must start with an uppercase letter, '_', or '\\', and contain only allowed characters.", - ParseError::UnclosedAnnotationArguments(_) => "the annotation arguments are missing a closing parenthesis ')'.", - ParseError::MalformedCodeBlock(_) => "the code block is malformed or incorrectly formatted.", - ParseError::InvalidComment(_) => "the comment is not a valid docblock. It should start with '/**' and end with '*/'.", - ParseError::InconsistentIndentation(_, _, _) => "the indentation in the docblock comment is inconsistent.", - ParseError::MissingAsterisk(_) => "an asterisk '*' is missing at the beginning of a line in the docblock.", - ParseError::MissingWhitespaceAfterAsterisk(_) => "missing whitespace after the asterisk '*' in the docblock.", - ParseError::MissingWhitespaceAfterOpeningAsterisk(_) => "missing whitespace after the opening '/**' in a single-line docblock.", - ParseError::MissingWhitespaceBeforeClosingAsterisk(_) => "missing whitespace before the closing '*/' in a single-line docblock.", - ParseError::ExpectedLine(_) => "a line or tag was expected in the docblock but none was found.", + ParseError::InvalidTrivia(_) => "The comment is not recognized as a docblock. It should start with '/**' and end with '*/'.", + ParseError::UnclosedInlineTag(_) => "The inline tag is missing a closing '}'.", + ParseError::UnclosedInlineCode(_) => "Inline code is missing a closing backtick '`'.", + ParseError::UnclosedCodeBlock(_) => "The code block is missing a closing delimiter '```'.", + ParseError::InvalidTagName(_) => "The tag name contains invalid characters.", + ParseError::InvalidAnnotationName(_) => "The annotation name is invalid. It must start with an uppercase letter, '_', or '\\', and contain only allowed characters.", + ParseError::UnclosedAnnotationArguments(_) => "The annotation arguments are missing a closing parenthesis ')'.", + ParseError::MalformedCodeBlock(_) => "The code block is malformed or incorrectly formatted.", + ParseError::InvalidComment(_) => "The comment is not a valid docblock. It should start with '/**' and end with '*/'.", + ParseError::InconsistentIndentation(_, _, _) => "The indentation in the docblock comment is inconsistent.", + ParseError::MissingAsterisk(_) => "An asterisk '*' is missing at the beginning of a line in the docblock.", + ParseError::MissingWhitespaceAfterAsterisk(_) => "Missing whitespace after the asterisk '*' in the docblock.", + ParseError::MissingWhitespaceAfterOpeningAsterisk(_) => "Missing whitespace after the opening '/**' in a single-line docblock.", + ParseError::MissingWhitespaceBeforeClosingAsterisk(_) => "Missing whitespace before the closing '*/' in a single-line docblock.", + ParseError::ExpectedLine(_) => "A line or tag was expected in the docblock but none was found.", } } pub fn help(&self) -> &'static str { match self { - ParseError::InvalidTrivia(_) => "replace the comment with a proper docblock starting with '/**' and ending with '*/'.", - ParseError::UnclosedInlineTag(_) => "add a closing '}' to complete the inline tag.", - ParseError::UnclosedInlineCode(_) => "add a closing '`' to terminate the inline code.", - ParseError::UnclosedCodeBlock(_) => "add a closing '```' to terminate the code block.", - ParseError::InvalidTagName(_) => "use only letters, numbers, and hyphens in the tag name.", - ParseError::InvalidAnnotationName(_) => "correct the annotation name to start with an uppercase letter, '_', or '\\', and use only letters, numbers, '_', '\\', or unicode characters.", - ParseError::UnclosedAnnotationArguments(_) => "add a closing ')' to complete the annotation arguments.", - ParseError::MalformedCodeBlock(_) => "ensure the code block starts with '```', optionally followed by a language identifier, and ends with a closing '```'.", - ParseError::InvalidComment(_) => "replace the comment with a valid docblock starting with '/**' and ending with '*/'.", - ParseError::InconsistentIndentation(_, _, _) => "adjust the indentation to be consistent across all lines in the docblock.", - ParseError::MissingAsterisk(_) => "add an '*' at the beginning of each line in the docblock after the opening '/**'.", - ParseError::MissingWhitespaceAfterAsterisk(_) => "insert a space after the '*' at the beginning of the line.", - ParseError::MissingWhitespaceAfterOpeningAsterisk(_) => "insert a space between '/**' and the text in the single-line docblock.", - ParseError::MissingWhitespaceBeforeClosingAsterisk(_) => "insert a space between the text and '*/' in the single-line docblock.", - ParseError::ExpectedLine(_) => "ensure that the docblock contains at least one line of text or a tag.", + ParseError::InvalidTrivia(_) => "Replace the comment with a proper docblock starting with '/**' and ending with '*/'.", + ParseError::UnclosedInlineTag(_) => "Add a closing '}' to complete the inline tag.", + ParseError::UnclosedInlineCode(_) => "Add a closing '`' to terminate the inline code.", + ParseError::UnclosedCodeBlock(_) => "Add a closing '```' to terminate the code block.", + ParseError::InvalidTagName(_) => "Use only letters, numbers, and hyphens in the tag name.", + ParseError::InvalidAnnotationName(_) => "Correct the annotation name to start with an uppercase letter, '_', or '\\', and use only letters, numbers, '_', '\\', or unicode characters.", + ParseError::UnclosedAnnotationArguments(_) => "Add a closing ')' to complete the annotation arguments.", + ParseError::MalformedCodeBlock(_) => "Ensure the code block starts with '```', optionally followed by a language identifier, and ends with a closing '```'.", + ParseError::InvalidComment(_) => "Replace the comment with a valid docblock starting with '/**' and ending with '*/'.", + ParseError::InconsistentIndentation(_, _, _) => "Adjust the indentation to be consistent across all lines in the docblock.", + ParseError::MissingAsterisk(_) => "Add an '*' at the beginning of each line in the docblock after the opening '/**'.", + ParseError::MissingWhitespaceAfterAsterisk(_) => "Insert a space after the '*' at the beginning of the line.", + ParseError::MissingWhitespaceAfterOpeningAsterisk(_) => "Insert a space between '/**' and the text in the single-line docblock.", + ParseError::MissingWhitespaceBeforeClosingAsterisk(_) => "Insert a space between the text and '*/' in the single-line docblock.", + ParseError::ExpectedLine(_) => "Ensure that the docblock contains at least one line of text or a tag.", } } } diff --git a/crates/lexer/src/error.rs b/crates/lexer/src/error.rs index c280784..b941cd5 100644 --- a/crates/lexer/src/error.rs +++ b/crates/lexer/src/error.rs @@ -28,11 +28,11 @@ impl HasSpan for SyntaxError { impl std::fmt::Display for SyntaxError { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { let message = match self { - Self::UnexpectedToken(token, _) => &format!("unexpected token `{}` (0x{:02X})", *token as char, token), - Self::UnrecognizedToken(token, _) => &format!("unrecognised token `{}` (0x{:02X})", *token as char, token), + Self::UnexpectedToken(token, _) => &format!("Unexpected token `{}` (0x{:02X})", *token as char, token), + Self::UnrecognizedToken(token, _) => &format!("Unrecognised token `{}` (0x{:02X})", *token as char, token), }; - write!(f, "syntax error: {}", message) + write!(f, "{}", message) } } @@ -43,6 +43,6 @@ impl From for Issue { let position = error.position(); let span = Span::new(position, Position { offset: position.offset + 1, ..position }); - Issue::error(error.to_string()).with_annotation(Annotation::primary(span)) + Issue::error(error.to_string()).with_annotation(Annotation::primary(span).with_message("Syntax error.")) } } diff --git a/crates/linter/src/plugin/best_practices/rules/combine_consecutive_issets.rs b/crates/linter/src/plugin/best_practices/rules/combine_consecutive_issets.rs index 80fc3e8..f7a80c4 100644 --- a/crates/linter/src/plugin/best_practices/rules/combine_consecutive_issets.rs +++ b/crates/linter/src/plugin/best_practices/rules/combine_consecutive_issets.rs @@ -34,11 +34,11 @@ impl<'a> Walker> for CombineConsecutiveIssetsRule { return; }; - let issue = Issue::new(context.level(), "consecutive isset calls can be combined") + let issue = Issue::new(context.level(), "Consecutive isset calls can be combined.") .with_annotation(Annotation::primary(left_isset.span())) .with_annotation(Annotation::primary(right_isset.span())) .with_annotation(Annotation::secondary(binary.span())) - .with_help("combine the isset calls into a single call, e.g. `isset($a, $b)`"); + .with_help("Combine the isset calls into a single call, e.g. `isset($a, $b)`."); // don't bother fixing if either of the isset calls is already parenthesized // this can be messy to fix and is not worth the effort. diff --git a/crates/linter/src/plugin/best_practices/rules/disallowed_functions.rs b/crates/linter/src/plugin/best_practices/rules/disallowed_functions.rs index 10b5af6..e4863eb 100644 --- a/crates/linter/src/plugin/best_practices/rules/disallowed_functions.rs +++ b/crates/linter/src/plugin/best_practices/rules/disallowed_functions.rs @@ -10,95 +10,76 @@ use crate::rule::Rule; #[derive(Clone, Debug)] pub struct DisallowedFunctionsRule; -impl DisallowedFunctionsRule { - fn report_disallowed_function(&self, function_call: &FunctionCall, context: &mut LintContext) { - let Expression::Identifier(identifier) = function_call.function.as_ref() else { - return; - }; - - let Some(disallowed_functions) = context.option("functions").and_then(|o| o.as_array()) else { - tracing::trace!("no disallowed functions found in configuration"); - - return; - }; - - let function_name = if context.is_name_imported(identifier) { - context.lookup_name(identifier) - } else { - let name = context.interner.lookup(&identifier.value()); - - if let Some(stripped) = name.strip_prefix('\\') { - stripped - } else { - name - } - }; - - if disallowed_functions.iter().any(|f| f.as_str().is_some_and(|f| f.eq(function_name))) { - let issue = Issue::new(context.level(), format!("disallowed function: `{}`", function_name)) - .with_annotation(Annotation::primary(function_call.span())) - .with_note(format!("The function `{}` is disallowed by your project configuration.", function_name)) - .with_help("use an alternative function or modify the configuration to allow this function."); +impl Rule for DisallowedFunctionsRule { + fn get_name(&self) -> &'static str { + "disallowed-functions" + } - context.report(issue); - } + fn get_default_level(&self) -> Option { + Some(Level::Warning) } +} - fn report_disallowed_extension_function(&self, function_call: &FunctionCall, context: &mut LintContext) { +impl<'a> Walker> for DisallowedFunctionsRule { + fn walk_in_function_call<'ast>(&self, function_call: &'ast FunctionCall, context: &mut LintContext<'a>) { let Expression::Identifier(identifier) = function_call.function.as_ref() else { return; }; - let Some(disallowed_extensions) = context.option("extensions").and_then(|o| o.as_array()) else { - tracing::trace!("no disallowed extensions found in configuration"); + let function_name = context.lookup_function_name(identifier); - return; - }; + // Check if the function is disallowed + if let Some(disallowed_functions) = context.option("functions").and_then(|o| o.as_array()) { + if disallowed_functions.iter().any(|f| f.as_str().is_some_and(|f| f.eq(function_name))) { + let issue = Issue::new(context.level(), format!("Function `{}` is disallowed.", function_name)) + .with_annotation( + Annotation::primary(function_call.span()) + .with_message(format!("Function `{}` is called here.`", function_name)), + ) + .with_note(format!("The function `{}` is disallowed by your project configuration.", function_name)) + .with_help("Use an alternative function or modify the configuration to allow this function."); - let function_name = context.lookup_function_name(identifier); + context.report(issue); - let Some(extension) = EXTENSION_FUNCTIONS.into_iter().find_map(|(extension, function_names)| { - if function_names.iter().any(|f| function_name.eq(*f)) { - Some(extension) - } else { - None + return; } - }) else { - // not an extension function - - return; + } else { + tracing::trace!("No disallowed functions found in configuration."); }; - if disallowed_extensions.iter().any(|e| e.as_str().is_some_and(|e| e.eq(extension))) { - let issue = Issue::new( - context.level(), - format!("disallowed extension function: `{}` from `{}` extension", function_name, extension), - ) - .with_annotation(Annotation::primary(function_call.span())) - .with_note(format!( - "functions from the `{}` extension are disallowed by your project configuration.", - extension - )) - .with_help("use an alternative function or modify the configuration to allow this extension."); - - context.report(issue); + // Check if the function is part of a disallowed extension + if let Some(disallowed_extensions) = context.option("extensions").and_then(|o| o.as_array()) { + let Some(extension) = EXTENSION_FUNCTIONS.into_iter().find_map(|(extension, function_names)| { + if function_names.iter().any(|f| function_name.eq(*f)) { + Some(extension) + } else { + None + } + }) else { + // not an extension function + + return; + }; + + if disallowed_extensions.iter().any(|e| e.as_str().is_some_and(|e| e.eq(extension))) { + let issue = Issue::new( + context.level(), + format!("Function `{}` from the `{}` extension is disallowed.", function_name, extension), + ) + .with_annotation( + Annotation::primary(function_call.span()) + .with_message(format!("Function `{}` is called here.", function_name)), + ) + .with_note(format!( + "Functions from the `{}` extension are disallowed by your project configuration.", + extension + )) + .with_help("Use an alternative function or modify the configuration to allow this extension."); + + context.report(issue); + } + } else { + tracing::trace!("No disallowed extensions found in configuration."); } } } - -impl Rule for DisallowedFunctionsRule { - fn get_name(&self) -> &'static str { - "disallowed-functions" - } - - fn get_default_level(&self) -> Option { - Some(Level::Warning) - } -} - -impl<'a> Walker> for DisallowedFunctionsRule { - fn walk_in_function_call<'ast>(&self, function_call: &'ast FunctionCall, context: &mut LintContext<'a>) { - self.report_disallowed_function(function_call, context); - self.report_disallowed_extension_function(function_call, context); - } -} diff --git a/crates/linter/src/plugin/best_practices/rules/excessive_nesting.rs b/crates/linter/src/plugin/best_practices/rules/excessive_nesting.rs index 40c874b..29cac7a 100644 --- a/crates/linter/src/plugin/best_practices/rules/excessive_nesting.rs +++ b/crates/linter/src/plugin/best_practices/rules/excessive_nesting.rs @@ -41,14 +41,14 @@ struct NestingWalker { impl NestingWalker { fn check_indent(&self, block: &Block, context: &mut LintContext) -> bool { if self.level > self.threshold { - let issue = Issue::new(context.level(), "excessive block nesting") + let issue = Issue::new(context.level(), "Excessive block nesting.") .with_annotation(Annotation::primary(block.span())) .with_note(format!( - "this block has a nesting level of {} which exceeds the threshold of {}", + "This block has a nesting level of {} which exceeds the threshold of {}.", self.level, self.threshold )) - .with_note("excessive nesting can make code harder to read, understand, and maintain.") - .with_help("refactor your code to reduce the level of nesting."); + .with_note("Excessive nesting can make code harder to read, understand, and maintain.") + .with_help("Refactor your code to reduce the level of nesting."); context.report(issue); diff --git a/crates/linter/src/plugin/best_practices/rules/loop_does_not_iterate.rs b/crates/linter/src/plugin/best_practices/rules/loop_does_not_iterate.rs index 52f5bf0..f4096c9 100644 --- a/crates/linter/src/plugin/best_practices/rules/loop_does_not_iterate.rs +++ b/crates/linter/src/plugin/best_practices/rules/loop_does_not_iterate.rs @@ -27,13 +27,13 @@ impl LoopDoesNotIterateRule { LoopTerminator::Return(return_stmt) => return_stmt.span(), }; - let issue = Issue::new(context.level(), "loop does not iterate") + let issue = Issue::new(context.level(), "Loop does not iterate.") .with_annotations([ - Annotation::primary(terminator_span) - .with_message("this statement unconditionally terminates the loop."), - Annotation::secondary(loop_span).with_message("this loop does not iterate."), + Annotation::primary(loop_span).with_message("This loop does not iterate."), + Annotation::secondary(terminator_span) + .with_message("This statement unconditionally terminates the loop."), ]) - .with_help("remove or refactor the loop to avoid redundant or misleading code."); + .with_help("Remove or refactor the loop to avoid redundant or misleading code."); context.report(issue); } diff --git a/crates/linter/src/plugin/best_practices/rules/no_debug_symbols.rs b/crates/linter/src/plugin/best_practices/rules/no_debug_symbols.rs index f545aa7..eb74720 100644 --- a/crates/linter/src/plugin/best_practices/rules/no_debug_symbols.rs +++ b/crates/linter/src/plugin/best_practices/rules/no_debug_symbols.rs @@ -85,10 +85,13 @@ impl<'a> Walker> for NoDebugSymbolsRule { let function_name = context.lookup_function_name(function_identifier); if DEBUG_FUNCTIONS.contains(&function_name) { - let issue = Issue::new(context.level(), format!("usage of debug function: `{}`", function_name)) - .with_annotation(Annotation::primary(function_call.span())) - .with_note("avoid using debug functions like `var_dump`, `print_r`, etc. in production code.") - .with_help("remove the debug function call."); + let issue = Issue::new(context.level(), format!("Usage of debug function `{}` detected.", function_name)) + .with_annotation( + Annotation::primary(function_call.span()) + .with_message(format!("Function `{}` is called here.", function_name)), + ) + .with_note("Avoid using debug functions like `var_dump`, `print_r`, etc. in production code.") + .with_help("Remove the debug function call."); context.report(issue); } diff --git a/crates/linter/src/plugin/best_practices/rules/no_empty_loop.rs b/crates/linter/src/plugin/best_practices/rules/no_empty_loop.rs index 5219780..15eaa35 100644 --- a/crates/linter/src/plugin/best_practices/rules/no_empty_loop.rs +++ b/crates/linter/src/plugin/best_practices/rules/no_empty_loop.rs @@ -24,10 +24,12 @@ impl NoEmptyLoopRule { fn report(&self, r#loop: impl HasSpan, context: &mut LintContext<'_>) { let loop_span = r#loop.span(); - let issue = Issue::new(context.level(), "loop body is empty") - .with_annotations([Annotation::primary(loop_span) - .with_message("this loop body is empty and does not perform any actions.")]) - .with_help("consider removing this loop or adding meaningful logic to its body."); + let issue = Issue::new(context.level(), "Loop body is empty") + .with_annotation( + Annotation::primary(loop_span) + .with_message("This loop body is empty and does not perform any actions."), + ) + .with_help("Consider removing this loop or adding meaningful logic to its body."); context.report_with_fix(issue, |plan| { plan.delete(loop_span.to_range(), SafetyClassification::PotentiallyUnsafe); diff --git a/crates/linter/src/plugin/best_practices/rules/no_goto.rs b/crates/linter/src/plugin/best_practices/rules/no_goto.rs index f73cde7..83f4e9d 100644 --- a/crates/linter/src/plugin/best_practices/rules/no_goto.rs +++ b/crates/linter/src/plugin/best_practices/rules/no_goto.rs @@ -21,27 +21,31 @@ impl Rule for NoGotoRule { impl<'a> Walker> for NoGotoRule { fn walk_in_goto<'ast>(&self, goto: &'ast Goto, context: &mut LintContext<'a>) { - let issue = Issue::new(context.level(), "avoid using `goto`") - .with_annotation(Annotation::primary(goto.goto.span())) + let issue = Issue::new(context.level(), "Avoid using `goto`.") + .with_annotation(Annotation::primary(goto.goto.span()).with_message("This `goto` statement is used here.")) .with_annotation(Annotation::secondary(goto.label.span())) - .with_note("the `goto` statement can make code harder to read, understand, and maintain.") - .with_note("it can lead to spaghetti code and make it difficult to follow the flow of execution.") + .with_note("The `goto` statement can make code harder to read, understand, and maintain.") + .with_note("It can lead to spaghetti code and make it difficult to follow the flow of execution.") .with_note( - "consider using structured control flow statements like `if`, `else`, `for`, and `while` instead.", + "Consider using structured control flow statements like `if`, `else`, `for`, and `while` instead.", ) - .with_help("refactor your code to avoid using `goto`."); + .with_help("Refactor your code to avoid using `goto`."); context.report(issue); } fn walk_in_label<'ast>(&self, label: &'ast Label, context: &mut LintContext<'a>) { - let issue = Issue::new(context.level(), "avoid using labels") - .with_annotation(Annotation::primary(label.span())) - .with_note("labels are often used with `goto` statements, which can make code harder to read and maintain.") + let name = context.lookup(&label.name.value); + + let issue = Issue::new(context.level(), "Avoid using labels") + .with_annotation( + Annotation::primary(label.span()).with_message(format!("Label `{}` is declared here.", name)), + ) + .with_note("Labels are often used with `goto` statements, which can make code harder to read and maintain.") .with_note( - "consider using structured control flow statements like `if`, `else`, `for`, and `while` instead.", + "Consider using structured control flow statements like `if`, `else`, `for`, and `while` instead.", ) - .with_help("refactor your code to avoid using labels."); + .with_help("Refactor your code to avoid using labels."); context.report(issue); } diff --git a/crates/linter/src/plugin/best_practices/rules/no_unused_parameter.rs b/crates/linter/src/plugin/best_practices/rules/no_unused_parameter.rs index 9ab6314..e11a7fd 100644 --- a/crates/linter/src/plugin/best_practices/rules/no_unused_parameter.rs +++ b/crates/linter/src/plugin/best_practices/rules/no_unused_parameter.rs @@ -33,13 +33,13 @@ impl NoUnusedParameterRule { return; } - let issue = Issue::new(context.level(), format!("unused parameter: `{}`", parameter_name)) + let issue = Issue::new(context.level(), format!("Parameter `{}` is never used.", parameter_name)) .with_annotations([ - Annotation::primary(parameter.span()), + Annotation::primary(parameter.span()).with_message(format!("Parameter `{}` is declared here.", parameter_name)), Annotation::secondary(function_like.span()), ]) - .with_note(format!("this parameter is declared but not used within the {}", kind)) - .with_help("consider prefixing the parameter with an underscore (`_`) to indicate that it is intentionally unused, or remove it if it is not needed"); + .with_note(format!("This parameter is declared but not used within the {}.", kind)) + .with_help("Consider prefixing the parameter with an underscore (`_`) to indicate that it is intentionally unused, or remove it if it is not needed."); context.report_with_fix(issue, |plan| { plan.insert( @@ -106,7 +106,7 @@ impl<'a> Walker> for NoUnusedParameterRule { }; if !context.option("methods").and_then(|o| o.as_bool()).unwrap_or(false) { - tracing::trace!("skipping checking parameters, methods are disabled"); + tracing::trace!("Skipping method parameters check because the rule is disabled for methods."); return; } diff --git a/crates/linter/src/plugin/best_practices/rules/use_while_instead_of_for.rs b/crates/linter/src/plugin/best_practices/rules/use_while_instead_of_for.rs index 58e09b6..a0c0e81 100644 --- a/crates/linter/src/plugin/best_practices/rules/use_while_instead_of_for.rs +++ b/crates/linter/src/plugin/best_practices/rules/use_while_instead_of_for.rs @@ -28,11 +28,11 @@ impl<'a> Walker> for UseWhileInsteadOfForRule { let issue = Issue::new( context.level(), - "use `while` instead of `for`", + "Use `while` loop instead of `for` loop.", ) - .with_annotation(Annotation::primary(r#for.span())) - .with_note("this `for` loop can be simplified to a `while` loop since it doesn't have initializations or increments.") - .with_help("use a `while` loop instead of a `for` loop."); + .with_annotation(Annotation::primary(r#for.span()).with_message("This `for` loop can be simplified to a `while` loop.")) + .with_note("This `for` loop can be simplified to a `while` loop since it doesn't have initializations or increments.") + .with_help("Use a `while` loop instead of a `for` loop."); context.report_with_fix(issue, |plan| { plan.delete(r#for.r#for.span.to_range(), SafetyClassification::Safe); diff --git a/crates/linter/src/plugin/comment/rules/no_shell_style.rs b/crates/linter/src/plugin/comment/rules/no_shell_style.rs index 59d9296..bfe6e00 100644 --- a/crates/linter/src/plugin/comment/rules/no_shell_style.rs +++ b/crates/linter/src/plugin/comment/rules/no_shell_style.rs @@ -29,9 +29,9 @@ impl<'a> Walker> for NoShellStyleRule { let comment_span = trivia.span(); let comment_pos = comment_span.start; - let issue = Issue::new(context.level(), "shell-style comments ('#') are not allowed.") - .with_annotation(Annotation::primary(comment_span).with_message("shell-style comment here")) - .with_help("consider using double slash comments ('//') instead."); + let issue = Issue::new(context.level(), "Shell-style comments ('#') are not allowed.") + .with_annotation(Annotation::primary(comment_span).with_message("This is a shell-style comment.")) + .with_help("Consider using double slash comments ('//') instead."); context.report_with_fix(issue, |plan| { plan.replace(comment_pos.range_for(1), "//", SafetyClassification::Safe); diff --git a/crates/linter/src/plugin/comment/rules/no_trailing_whitespace.rs b/crates/linter/src/plugin/comment/rules/no_trailing_whitespace.rs index 082f47d..b5f2295 100644 --- a/crates/linter/src/plugin/comment/rules/no_trailing_whitespace.rs +++ b/crates/linter/src/plugin/comment/rules/no_trailing_whitespace.rs @@ -49,14 +49,14 @@ impl<'a> Walker> for NoTrailingWhitespaceRule { ); issues.push( - Issue::new(context.level(), "trailing whitespace detected in comment.") + Issue::new(context.level(), "Trailing whitespace detected in comment.") .with_annotations([ - Annotation::primary(whitespace_span).with_message("trailing whitespace"), + Annotation::primary(whitespace_span).with_message("Trailing whitespace detected."), Annotation::secondary(comment_span) - .with_message("comment containing trailing whitespace"), + .with_message("Comment with trailing whitespace."), ]) - .with_note("trailing whitespaces can cause unnecessary diffs and formatting issues.") - .with_help("remove the extra whitespace.") + .with_note("Trailing whitespaces can cause unnecessary diffs and formatting issues.") + .with_help("Remove the extra whitespace.") .with_suggestion(whitespace_span.source(), { let mut plan = FixPlan::new(); diff --git a/crates/linter/src/plugin/comment/rules/no_untagged_fixme.rs b/crates/linter/src/plugin/comment/rules/no_untagged_fixme.rs index 4a05c3d..14222e9 100644 --- a/crates/linter/src/plugin/comment/rules/no_untagged_fixme.rs +++ b/crates/linter/src/plugin/comment/rules/no_untagged_fixme.rs @@ -47,10 +47,10 @@ impl<'a> Walker> for NoUntaggedFixmeRule { } context.report( - Issue::new(context.level(), "FIXME should be tagged with (@username) or (#issue)") + Issue::new(context.level(), "FIXME comment should be tagged with (@username) or (#issue).") .with_annotation(Annotation::primary(trivia.span)) .with_help( - "add a user tag or issue reference to the FIXME comment, e.g. FIXME(@azjezz), FIXME(azjezz), FIXME(#123)", + "Add a user tag or issue reference to the FIXME comment, e.g. FIXME(@azjezz), FIXME(azjezz), FIXME(#123).", ) ); diff --git a/crates/linter/src/plugin/comment/rules/no_untagged_todo.rs b/crates/linter/src/plugin/comment/rules/no_untagged_todo.rs index c984def..65e6907 100644 --- a/crates/linter/src/plugin/comment/rules/no_untagged_todo.rs +++ b/crates/linter/src/plugin/comment/rules/no_untagged_todo.rs @@ -47,10 +47,10 @@ impl<'a> Walker> for NoUntaggedTodoRule { } context.report( - Issue::new(context.level(), "TODO should be tagged with (@username) or (#issue)") + Issue::new(context.level(), "TODO should be tagged with (@username) or (#issue).") .with_annotation(Annotation::primary(trivia.span)) .with_help( - "add a user tag or issue reference to the TODO comment, e.g. TODO(@azjezz), TODO(azjezz), TODO(#123)", + "Add a user tag or issue reference to the TODO comment, e.g. TODO(@azjezz), TODO(azjezz), TODO(#123).", ) ); diff --git a/crates/linter/src/plugin/consistency/rules/array_syntax.rs b/crates/linter/src/plugin/consistency/rules/array_syntax.rs index 58f25bd..bdc8f5a 100644 --- a/crates/linter/src/plugin/consistency/rules/array_syntax.rs +++ b/crates/linter/src/plugin/consistency/rules/array_syntax.rs @@ -26,9 +26,11 @@ impl<'a> Walker> for ArraySyntaxRule { return; } - let issue = Issue::new(context.level(), "short array syntax `[..]` is preferred over `array(..)`") - .with_annotation(Annotation::primary(arr.span())) - .with_help("use the short array syntax `[..]` instead"); + let issue = Issue::new(context.level(), "Short array syntax `[..]` is preferred over `array(..)`.") + .with_annotation( + Annotation::primary(arr.span()).with_message("This array uses the long array syntax `array(..)`."), + ) + .with_help("Use the short array syntax `[..]` instead"); context.report_with_fix(issue, |plan| { plan.replace(arr.array.span.join(arr.left_parenthesis).to_range(), "[", SafetyClassification::Safe); @@ -41,9 +43,11 @@ impl<'a> Walker> for ArraySyntaxRule { return; } - let issue = Issue::new(context.level(), "long array syntax `array(..)` is preferred over `[..]`") - .with_annotation(Annotation::primary(arr.span())) - .with_help("use the long array syntax `array(..)` instead"); + let issue = Issue::new(context.level(), "Long array syntax `array(..)` is preferred over `[..]`.") + .with_annotation( + Annotation::primary(arr.span()).with_message("This array uses the short array syntax `[..]`."), + ) + .with_help("Use the long array syntax `array(..)` instead"); context.report_with_fix(issue, |plan| { plan.replace(arr.left_bracket.to_range(), "array(", SafetyClassification::Safe); diff --git a/crates/linter/src/plugin/consistency/rules/lowercase_hint.rs b/crates/linter/src/plugin/consistency/rules/lowercase_hint.rs index 29d3637..84db2de 100644 --- a/crates/linter/src/plugin/consistency/rules/lowercase_hint.rs +++ b/crates/linter/src/plugin/consistency/rules/lowercase_hint.rs @@ -35,9 +35,9 @@ impl<'a> Walker> for LowercaseHintRule { let name = context.interner.lookup(&identifier.value); let lowered = name.to_ascii_lowercase(); if !lowered.eq(&name) { - let issue = Issue::new(context.level(), format!("type hint `{}` should be in lowercase", name)) + let issue = Issue::new(context.level(), format!("Type hint `{}` should be in lowercase.", name)) .with_annotation(Annotation::primary(identifier.span())) - .with_help(format!("consider using `{}` instead of `{}`.", lowered, name)); + .with_help(format!("Consider using `{}` instead of `{}`.", lowered, name)); context.report_with_fix(issue, |p| { p.replace(identifier.span.to_range(), lowered, SafetyClassification::Safe) diff --git a/crates/linter/src/plugin/consistency/rules/lowercase_keyword.rs b/crates/linter/src/plugin/consistency/rules/lowercase_keyword.rs index e0d7346..c3fa517 100644 --- a/crates/linter/src/plugin/consistency/rules/lowercase_keyword.rs +++ b/crates/linter/src/plugin/consistency/rules/lowercase_keyword.rs @@ -25,10 +25,10 @@ impl<'a> Walker> for LowercaseKeywordRule { let name = context.lookup(&keyword.value); let lowered = name.to_ascii_lowercase(); if !lowered.eq(&name) { - let issue = Issue::new(context.level(), format!("keyword `{}` should be in lowercase", name)) + let issue = Issue::new(context.level(), format!("Keyword `{}` should be in lowercase.", name)) .with_annotation(Annotation::primary(keyword.span())) - .with_note(format!("the keyword `{}` does not follow lowercase convention.", name)) - .with_help(format!("consider using `{}` instead of `{}`.", lowered, name)); + .with_note(format!("The keyword `{}` does not follow lowercase convention.", name)) + .with_help(format!("Consider using `{}` instead of `{}`.", lowered, name)); context.report_with_fix(issue, |p| p.replace(keyword.span.to_range(), lowered, SafetyClassification::Safe)); } diff --git a/crates/linter/src/plugin/consistency/rules/no_function_aliases.rs b/crates/linter/src/plugin/consistency/rules/no_function_aliases.rs index a123bda..764e5b8 100644 --- a/crates/linter/src/plugin/consistency/rules/no_function_aliases.rs +++ b/crates/linter/src/plugin/consistency/rules/no_function_aliases.rs @@ -135,37 +135,37 @@ impl<'a> Walker> for NoFunctionAliasesRule { if alias_used_in_code == original_name { // Special case: imported alias as the original function format!( - "function `{}` refers to alias function `{}`, which should not be used", + "Function `{}` refers to alias function `{}`, which should not be used.", alias_used_in_code, resolved_name ) } else { format!( - "function alias `{}` (imported as `{}`) should not be used", + "Function alias `{}` (imported as `{}`) should not be used.", resolved_name, alias_used_in_code ) } } else { - format!("function alias `{}` should not be used", resolved_name) + format!("Function alias `{}` should not be used.", resolved_name) }, ) .with_annotation(Annotation::primary(identifier.span()).with_message(if is_name_imported { if alias_used_in_code == original_name { // Special case: imported alias as the original function format!( - "function `{}` refers to alias function `{}`, which should not be used", + "Function `{}` refers to alias function `{}`, which should not be used.", alias_used_in_code, resolved_name ) } else { format!( - "function alias `{}` (imported as `{}`) should not be used", + "Function alias `{}` (imported as `{}`) should not be used.", resolved_name, alias_used_in_code ) } } else { - format!("function alias `{}` should not be used", resolved_name) + format!("Function alias `{}` should not be used.", resolved_name) })) - .with_note(format!("the function `{}` is an alias of `{}`.", resolved_name, original_name)) - .with_help(format!("consider using the function `{}` instead.", original_name)); + .with_note(format!("The function `{}` is an alias of `{}`.", resolved_name, original_name)) + .with_help(format!("Consider using the function `{}` instead.", original_name)); if is_name_imported { if alias_used_in_code != resolved_name { @@ -173,10 +173,10 @@ impl<'a> Walker> for NoFunctionAliasesRule { } else { // Special case: imported alias as the original function, e.g `use function i_am_the_alias as original_func;` issue = issue.with_note(format!( - "you are importing the alias function `{}` as `{}`.", + "You are importing the alias function `{}` as `{}`.", resolved_name, alias_used_in_code )); - issue = issue.with_note(format!("consider importing `{}` instead.", original_name)); + issue = issue.with_note(format!("Consider importing `{}` instead.", original_name)); } } @@ -193,7 +193,7 @@ impl<'a> Walker> for NoFunctionAliasesRule { // current namespace, so we mark it as unsafe. // // TODO(azjezz): this case can be considered safe is we are in the global namespace. - SafetyClassification::Unsafe + SafetyClassification::PotentiallyUnsafe }, ) }); diff --git a/crates/linter/src/plugin/consistency/rules/no_tag_pair_terminator.rs b/crates/linter/src/plugin/consistency/rules/no_tag_pair_terminator.rs index d7079d2..be2419c 100644 --- a/crates/linter/src/plugin/consistency/rules/no_tag_pair_terminator.rs +++ b/crates/linter/src/plugin/consistency/rules/no_tag_pair_terminator.rs @@ -26,9 +26,12 @@ impl<'a> Walker> for NoTagPairTerminatorRule { return; }; - let issue = Issue::new(context.level(), "semicolon terminator is preferred over tag-pair terminator") - .with_annotation(Annotation::primary(close.span().join(open.span()))) - .with_help("replace `?> Walker> for OptionalParameterBeforeRequiredRule { let issue = Issue::new( context.level(), format!( - "optional parameter(s) `{}` defined before required parameter `{}`", + "Optional parameter(s) `{}` defined before required parameter `{}`.", optional_parameters .iter() .map(|(opt_name, _, _)| context.lookup(opt_name).to_string()) @@ -50,15 +50,15 @@ impl<'a> Walker> for OptionalParameterBeforeRequiredRule { ) .with_annotation( Annotation::primary(parameter.variable.span()) - .with_message(format!("required parameter `{}` defined here", name)), + .with_message(format!("Required parameter `{}` defined here", name)), ) .with_annotations(optional_parameters.iter().map(|(opt_name, opt_span, _)| { Annotation::secondary(*opt_span) - .with_message(format!("optional parameter `{}` defined here", context.lookup(opt_name))) + .with_message(format!("Optional parameter `{}` defined here.", context.lookup(opt_name))) })) - .with_note("parameters after an optional one are implicitly required") - .with_note("defining optional parameters before required ones has been deprecated since PHP 8.0.") - .with_help("move all optional parameters to the end of the parameter list to resolve this issue."); + .with_note("Parameters after an optional one are implicitly required.") + .with_note("Defining optional parameters before required ones has been deprecated since PHP 8.0.") + .with_help("Move all optional parameters to the end of the parameter list to resolve this issue."); context.report_with_fix(issue, |plan| { for (_, _, default_span) in &optional_parameters { diff --git a/crates/linter/src/plugin/deprecation/rules/php82/return_by_reference_from_void_function.rs b/crates/linter/src/plugin/deprecation/rules/php82/return_by_reference_from_void_function.rs index 51cb3d2..ad650ef 100644 --- a/crates/linter/src/plugin/deprecation/rules/php82/return_by_reference_from_void_function.rs +++ b/crates/linter/src/plugin/deprecation/rules/php82/return_by_reference_from_void_function.rs @@ -101,15 +101,18 @@ impl<'a> Walker> for ReturnByReferenceFromVoidFunctionRule { fn report(context: &mut LintContext<'_>, kind: &'static str, span: Span, ampersand: &Span, is_set_hook: bool) { let message = if !is_set_hook { - format!("returning by reference from a void {} is deprecated since PHP 8.0", kind) + format!("Returning by reference from a void {} is deprecated since PHP 8.0.", kind) } else { - "returning by reference from a set property hook is deprecated since PHP 8.0".to_string() + "Returning by reference from a set property hook is deprecated since PHP 8.0".to_string() }; let issue = Issue::new(context.level(), message) - .with_annotation(Annotation::primary(*ampersand).with_message("`&` indicating a return by reference")) + .with_annotation( + Annotation::primary(*ampersand) + .with_message(format!("The `&` indicates that the {} returns by reference.", kind)), + ) .with_annotation(Annotation::secondary(span)) - .with_help("consider removing the `&` to comply with PHP 8.0 standards and avoid future issues.".to_string()); + .with_help("Consider removing the `&` to comply with PHP 8.0 standards and avoid future issues.".to_string()); context.report_with_fix(issue, |plan| { plan.delete(ampersand.to_range(), SafetyClassification::Safe); diff --git a/crates/linter/src/plugin/deprecation/rules/php84/implicitly_nullable_parameter.rs b/crates/linter/src/plugin/deprecation/rules/php84/implicitly_nullable_parameter.rs index 0b5b94b..d14b643 100644 --- a/crates/linter/src/plugin/deprecation/rules/php84/implicitly_nullable_parameter.rs +++ b/crates/linter/src/plugin/deprecation/rules/php84/implicitly_nullable_parameter.rs @@ -53,16 +53,17 @@ impl<'a> Walker> for ImplicitlyNullableParameterRule { let issue = Issue::new( context.level(), - format!("parameter `{}` is implicitly nullable and relies on a deprecated feature", parameter_name), + format!("Parameter `{}` is implicitly nullable and relies on a deprecated feature.", parameter_name), ) .with_annotation( - Annotation::primary(function_like_parameter.span()).with_message("implicitly nullable parameter"), + Annotation::primary(function_like_parameter.span()) + .with_message(format!("Parameter `{}` is declared here.", parameter_name)), ) .with_help(format!( - "consider using an explicit nullable type hint ( `{}` ) or replacing the default value.", + "Consider using an explicit nullable type hint ( `{}` ) or replacing the default value.", replacement_hint )) - .with_note("updating this will future-proof your code and align it with PHP 8.4 standards."); + .with_note("Updating this will future-proof your code and align it with PHP 8.4 standards."); context.report_with_fix(issue, |plan| { plan.replace(hint.span().to_range(), replacement_hint, SafetyClassification::Safe); diff --git a/crates/linter/src/plugin/deprecation/rules/php84/underscore_classname.rs b/crates/linter/src/plugin/deprecation/rules/php84/underscore_classname.rs index 53e0add..d487825 100644 --- a/crates/linter/src/plugin/deprecation/rules/php84/underscore_classname.rs +++ b/crates/linter/src/plugin/deprecation/rules/php84/underscore_classname.rs @@ -26,11 +26,11 @@ impl<'a> Walker> for UnderscoreClassNameRule { return; } - let issue = Issue::new(context.level(), "using `_` as a class name is deprecated") + let issue = Issue::new(context.level(), "Using `_` as a class name is deprecated.") .with_annotation( - Annotation::primary(class.name.span()).with_message("rename the class to something more descriptive"), + Annotation::primary(class.name.span()).with_message("Rename the class to something more descriptive."), ) - .with_note("class names consisting only of `_` are deprecated. consider using a meaningful name."); + .with_note("Class names consisting only of `_` are deprecated. consider using a meaningful name."); context.report(issue); } @@ -41,12 +41,12 @@ impl<'a> Walker> for UnderscoreClassNameRule { return; } - let issue = Issue::new(context.level(), "using `_` as an interface name is deprecated") + let issue = Issue::new(context.level(), "Using `_` as an interface name is deprecated.") .with_annotation( Annotation::primary(interface.name.span()) - .with_message("rename the interface to something more descriptive"), + .with_message("Rename the interface to something more descriptive."), ) - .with_note("interface names consisting only of `_` are deprecated. consider using a meaningful name."); + .with_note("Interface names consisting only of `_` are deprecated. consider using a meaningful name."); context.report(issue); } @@ -57,11 +57,12 @@ impl<'a> Walker> for UnderscoreClassNameRule { return; } - let issue = Issue::new(context.level(), "using `_` as a trait name is deprecated") + let issue = Issue::new(context.level(), "Using `_` as a trait name is deprecated.") .with_annotation( - Annotation::primary(r#trait.name.span()).with_message("rename the trait to something more descriptive"), + Annotation::primary(r#trait.name.span()) + .with_message("Rename the trait to something more descriptive."), ) - .with_note("trait names consisting only of `_` are deprecated. consider using a meaningful name."); + .with_note("Trait names consisting only of `_` are deprecated. consider using a meaningful name."); context.report(issue); } @@ -72,11 +73,11 @@ impl<'a> Walker> for UnderscoreClassNameRule { return; } - let issue = Issue::new(context.level(), "using `_` as an enum name is deprecated") + let issue = Issue::new(context.level(), "Using `_` as an enum name is deprecated.") .with_annotation( - Annotation::primary(r#enum.name.span()).with_message("rename the enum to something more descriptive"), + Annotation::primary(r#enum.name.span()).with_message("Rename the enum to something more descriptive."), ) - .with_note("enum names consisting only of `_` are deprecated. consider using a meaningful name."); + .with_note("Enum names consisting only of `_` are deprecated. consider using a meaningful name."); context.report(issue); } diff --git a/crates/linter/src/plugin/laravel/rules/safety/no_request_all.rs b/crates/linter/src/plugin/laravel/rules/safety/no_request_all.rs index 1e7316c..857c5ab 100644 --- a/crates/linter/src/plugin/laravel/rules/safety/no_request_all.rs +++ b/crates/linter/src/plugin/laravel/rules/safety/no_request_all.rs @@ -68,18 +68,19 @@ impl<'a> Walker> for NoRequestAllRule { || context.lookup(&identifier.value()).eq_ignore_ascii_case(REQUEST_CLASS) } _ => { - // we do not care abotu closure creation.. + // we do not care about closure creation.. false } } }); for reference in request_all_references { - let issue = Issue::new(context.level(), "avoid using `$request->all()` or `Request::all()`.") - .with_annotations([ - Annotation::primary(reference.span()).with_message("using `$request->all()` retrieves all input values, including ones you might not expect or intend to handle.") - ]) - .with_help("use `$request->only([...])` to specify the inputs you need explicitly, ensuring better security and validation."); + let issue = Issue::new(context.level(), "Avoid using `$request->all()` or `Request::all()`.") + .with_annotation( + Annotation::primary(reference.span()).with_message("`Request::all()` is called here") + ) + .with_note("Using `$request->all()` retrieves all input values, including ones you might not expect or intend to handle.") + .with_help("Use `$request->only([...])` to specify the inputs you need explicitly, ensuring better security and validation."); context.report(issue); } diff --git a/crates/linter/src/plugin/migration/rules/php80/str_contains.rs b/crates/linter/src/plugin/migration/rules/php80/str_contains.rs index 23f7c7a..ea61c0c 100644 --- a/crates/linter/src/plugin/migration/rules/php80/str_contains.rs +++ b/crates/linter/src/plugin/migration/rules/php80/str_contains.rs @@ -58,12 +58,11 @@ impl<'a> Walker> for StrContainsRule { let issue = Issue::new( context.level(), - "consider replacing `strpos` with `str_contains` for improved readability and intent clarity", + "Consider replacing `strpos` with `str_contains` for improved readability and intent clarity.", ) - .with_annotation(Annotation::primary(call.span())) - .with_annotation(Annotation::secondary(binary.span())) + .with_annotation(Annotation::primary(binary.span()).with_message("This comparison can be simplified.")) .with_help("`strpos($a, $b) !== false` can be simplified to `str_contains($a, $b)`.") - .with_note("using `str_contains` makes the code easier to understand and more expressive."); + .with_note("Using `str_contains` makes the code easier to understand and more expressive."); context.report_with_fix(issue, |plan| { // Mark the fix as potentially unsafe due to possible redefinition of `strpos` in the namespace. diff --git a/crates/linter/src/plugin/migration/rules/php80/str_starts_with.rs b/crates/linter/src/plugin/migration/rules/php80/str_starts_with.rs index d8f7ef9..c35692e 100644 --- a/crates/linter/src/plugin/migration/rules/php80/str_starts_with.rs +++ b/crates/linter/src/plugin/migration/rules/php80/str_starts_with.rs @@ -59,12 +59,11 @@ impl<'a> Walker> for StrStartsWithRule { let issue = Issue::new( context.level(), - "consider replacing `strpos` with `str_starts_with` for improved readability and intent clarity", + "Consider replacing `strpos` with `str_starts_with` for improved readability and intent clarity.", ) - .with_annotation(Annotation::primary(call.span())) - .with_annotation(Annotation::secondary(binary.span())) + .with_annotation(Annotation::secondary(binary.span()).with_message("This expression can be simplified.")) .with_help("`strpos($a, $b) === 0` can be simplified to `str_starts_with($a, $b)`.") - .with_note("using `str_starts_with` makes the code easier to understand and more expressive."); + .with_note("Using `str_starts_with` makes the code easier to understand and more expressive."); context.report_with_fix(issue, |plan| { // we can't guarantee that the replacement is safe, since `strpos` can be re-defined in diff --git a/crates/linter/src/plugin/migration/rules/php81/explicit_octal_notation.rs b/crates/linter/src/plugin/migration/rules/php81/explicit_octal_notation.rs index d1c4a9f..06d147a 100644 --- a/crates/linter/src/plugin/migration/rules/php81/explicit_octal_notation.rs +++ b/crates/linter/src/plugin/migration/rules/php81/explicit_octal_notation.rs @@ -34,13 +34,13 @@ impl<'a> Walker> for ExplicitOctalNotationRule { return; } - let issue = Issue::new(context.level(), "use explicit octal numeral notation") + let issue = Issue::new(context.level(), "Use explicit octal numeral notation.") .with_annotation( Annotation::primary(literal_integer.span()) - .with_message("implicit octal numeral notation") + .with_message("Implicit octal numeral notation used here."), ) - .with_note("using `0o` makes the octal intent explicit and avoids confusion with other formats.") - .with_help("replace the leading `0` with `0o` to make the octal intent explicit") + .with_note("Using `0o` makes the octal intent explicit and avoids confusion with other formats.") + .with_help("Replace the leading `0` with `0o` to make the octal intent explicit") .with_link("https://www.php.net/manual/en/migration81.new-features.php#migration81.new-features.core.octal-literal-prefix") ; diff --git a/crates/linter/src/plugin/migration/rules/php82/readonly_class_promotion.rs b/crates/linter/src/plugin/migration/rules/php82/readonly_class_promotion.rs index bc13cda..5ebbd70 100644 --- a/crates/linter/src/plugin/migration/rules/php82/readonly_class_promotion.rs +++ b/crates/linter/src/plugin/migration/rules/php82/readonly_class_promotion.rs @@ -58,11 +58,13 @@ impl<'a> Walker> for ReadonlyClassPromotionRule { .collect::>(); // Prepare fix plan - let issue = Issue::new(context.level(), "promote class to readonly") + let issue = Issue::new(context.level(), "Promote class to readonly") .with_annotations(annotations) - .with_annotation(Annotation::primary(class.span()).with_message("class has all readonly properties")) - .with_note("classes with all readonly properties can be marked readonly themselves.") - .with_help("add the `readonly` modifier to the class and remove `readonly` from all properties"); + .with_annotation( + Annotation::primary(class.span()).with_message("This class contains only readonly properties."), + ) + .with_note("Classes that contains only readonly properties can be marked readonly themselves.") + .with_help("Add the `readonly` modifier to the class and remove `readonly` from all properties"); // Determine safety classification let safety = if class.extends.is_some() { diff --git a/crates/linter/src/plugin/naming/rules/class.rs b/crates/linter/src/plugin/naming/rules/class.rs index 02b8eed..d7ef125 100644 --- a/crates/linter/src/plugin/naming/rules/class.rs +++ b/crates/linter/src/plugin/naming/rules/class.rs @@ -27,14 +27,14 @@ impl<'a> Walker> for ClassRule { let fqcn = context.lookup_name(&class.name); if !mago_casing::is_class_case(name) { - let issue = Issue::new(context.level(), format!("class name `{}` should be in class case", name)) + let issue = Issue::new(context.level(), format!("Class name `{}` should be in class case.", name)) .with_annotations([ - Annotation::primary(class.name.span()), - Annotation::secondary(class.span()).with_message(format!("class `{}` is declared here", fqcn)), + Annotation::primary(class.name.span()).with_message("Class `{}` is declared here.".to_string()), + Annotation::secondary(class.span()).with_message(format!("Class `{}` is defined here.", fqcn)), ]) - .with_note(format!("the class name `{}` does not follow class naming convention.", name)) + .with_note(format!("The class name `{}` does not follow class naming convention.", name)) .with_help(format!( - "consider renaming it to `{}` to adhere to the naming convention.", + "Consider renaming it to `{}` to adhere to the naming convention.", mago_casing::to_class_case(name) )); @@ -45,18 +45,19 @@ impl<'a> Walker> for ClassRule { && context.option("psr").and_then(|o| o.as_bool()).unwrap_or(true) && !name.starts_with("Abstract") { + let suggested_name = format!("Abstract{}", mago_casing::to_class_case(name)); + issues.push( Issue::new( context.level(), - format!("abstract class name `{}` should be prefixed with `Abstract`", name), + format!("Abstract class name `{}` should be prefixed with `Abstract`.", name), ) .with_annotations([ - Annotation::primary(class.name.span), - Annotation::secondary(class.span()) - .with_message(format!("abstract class `{}` is declared here", fqcn)), + Annotation::primary(class.name.span()).with_message("Class `{}` is declared here.".to_string()), + Annotation::secondary(class.span()).with_message(format!("Class `{}` is defined here.", fqcn)), ]) - .with_note(format!("the abstract class name `{}` does not follow PSR naming convention.", name)) - .with_help(format!("consider renaming it to `Abstract{}` to adhere to the naming convention.", name)), + .with_note(format!("The abstract class name `{}` does not follow PSR naming convention.", name)) + .with_help(format!("Consider renaming it to `{}` to adhere to the naming convention.", suggested_name)), ); } diff --git a/crates/linter/src/plugin/naming/rules/constant.rs b/crates/linter/src/plugin/naming/rules/constant.rs index 951cf77..8834dfc 100644 --- a/crates/linter/src/plugin/naming/rules/constant.rs +++ b/crates/linter/src/plugin/naming/rules/constant.rs @@ -26,15 +26,18 @@ impl<'a> Walker> for ConstantRule { let fqcn = context.lookup_name(&item.name); if !mago_casing::is_constant_case(name) { context.report( - Issue::new(context.level(), format!("constant name `{}` should be in constant case", name)) - .with_annotations([ - Annotation::primary(item.name.span()), + Issue::new(context.level(), format!("Constant name `{}` should be in constant case.", name)) + .with_annotation( + Annotation::primary(item.name.span()) + .with_message(format!("Constant item `{}` is declared here.", name)), + ) + .with_annotation( Annotation::secondary(constant.span()) - .with_message(format!("constant `{}` is declared here", fqcn)), - ]) - .with_note(format!("the constant name `{}` does not follow constant naming convention.", name)) + .with_message(format!("Constant `{}` is defined here.", fqcn)), + ) + .with_note(format!("The constant name `{}` does not follow constant naming convention.", name)) .with_help(format!( - "consider renaming it to `{}` to adhere to the naming convention.", + "Consider renaming it to `{}` to adhere to the naming convention.", mago_casing::to_constant_case(name) )), ); @@ -58,25 +61,28 @@ impl<'a> Walker> for ConstantRule { Issue::new( context.level(), format!( - "{} constant name `{}::{}` should be in constant case", + "{} constant name `{}::{}` should be in constant case.", class_like_kind, class_like_name, name ), ) - .with_annotations([ - Annotation::primary(item.name.span()), - Annotation::secondary(class_like_constant.span()).with_message(format!( - "{} constant `{}::{}` is declared here", - class_like_kind, class_like_name, name - )), + .with_annotation( + Annotation::primary(item.name.span()) + .with_message(format!("Constant item `{}` is declared here.", name)), + ) + .with_annotation(Annotation::secondary(class_like_constant.span()).with_message(format!( + "{} constant `{}::{}` is defined here.", + class_like_kind, class_like_fqcn, name + ))) + .with_annotation( Annotation::secondary(class_like_span) - .with_message(format!("{} `{}` is declared here", class_like_kind, class_like_fqcn)), - ]) + .with_message(format!("{} `{}` is defined here.", class_like_kind, class_like_fqcn)), + ) .with_note(format!( - "the {} constant name `{}::{}` does not follow constant naming convention.", + "The {} constant name `{}::{}` does not follow constant naming convention.", class_like_kind, class_like_name, name )) .with_help(format!( - "consider renaming it to `{}` to adhere to the naming convention.", + "Consider renaming it to `{}` to adhere to the naming convention.", mago_casing::to_constant_case(name) )), ); diff --git a/crates/linter/src/plugin/naming/rules/enum.rs b/crates/linter/src/plugin/naming/rules/enum.rs index 930b9c2..92cac5d 100644 --- a/crates/linter/src/plugin/naming/rules/enum.rs +++ b/crates/linter/src/plugin/naming/rules/enum.rs @@ -26,14 +26,17 @@ impl<'a> Walker> for EnumRule { if !mago_casing::is_class_case(name) { context.report( - Issue::new(context.level(), format!("enum name `{}` should be in class case", name)) - .with_annotations([ - Annotation::primary(r#enum.name.span()), - Annotation::secondary(r#enum.span()).with_message(format!("enum `{}` is declared here", fqcn)), - ]) - .with_note(format!("the enum name `{}` does not follow class naming convention.", name)) + Issue::new(context.level(), format!("Enum name `{}` should be in class case.", name)) + .with_annotation( + Annotation::primary(r#enum.name.span()) + .with_message(format!("Enum `{}` is declared here.", name)), + ) + .with_annotation( + Annotation::secondary(r#enum.span()).with_message(format!("Enum `{}` is defined here.", fqcn)), + ) + .with_note(format!("The enum name `{}` does not follow class naming convention.", name)) .with_help(format!( - "consider renaming it to `{}` to adhere to the naming convention.", + "Consider renaming it to `{}` to adhere to the naming convention.", mago_casing::to_class_case(name) )), ); diff --git a/crates/linter/src/plugin/naming/rules/function.rs b/crates/linter/src/plugin/naming/rules/function.rs index edd2028..1e1093b 100644 --- a/crates/linter/src/plugin/naming/rules/function.rs +++ b/crates/linter/src/plugin/naming/rules/function.rs @@ -31,19 +31,22 @@ impl Walker> for FunctionRule { context.report( Issue::new( context.level(), - format!("function name `{}` should be in either camel case or snake case", name), + format!("Function name `{}` should be in either camel case or snake case.", name), ) - .with_annotations([ - Annotation::primary(function.name.span()), + .with_annotation( + Annotation::primary(function.name.span()) + .with_message(format!("Function `{}` is declared here.`", name)), + ) + .with_annotation( Annotation::secondary(function.span()) - .with_message(format!("function `{}` is declared here", fqfn)), - ]) + .with_message(format!("Function `{}` is defined here.", fqfn)), + ) .with_note(format!( - "the function name `{}` does not follow either camel case or snake naming convention.", + "The function name `{}` does not follow either camel case or snake naming convention.", name )) .with_help(format!( - "consider renaming it to `{}` or `{}` to adhere to the naming convention.", + "Consider renaming it to `{}` or `{}` to adhere to the naming convention.", mago_casing::to_camel_case(name), mago_casing::to_snake_case(name) )), @@ -56,15 +59,18 @@ impl Walker> for FunctionRule { if camel_case { if !mago_casing::is_camel_case(name) { context.report( - Issue::new(context.level(), format!("function name `{}` should be in camel case", name)) - .with_annotations([ - Annotation::primary(function.name.span()), + Issue::new(context.level(), format!("Function name `{}` should be in camel case.", name)) + .with_annotation( + Annotation::primary(function.name.span()) + .with_message(format!("Function `{}` is declared here.`", name)), + ) + .with_annotation( Annotation::secondary(function.span()) - .with_message(format!("function `{}` is declared here", fqfn)), - ]) - .with_note(format!("the function name `{}` does not follow camel naming convention.", name)) + .with_message(format!("Function `{}` is defined here.", fqfn)), + ) + .with_note(format!("The function name `{}` does not follow camel naming convention.", name)) .with_help(format!( - "consider renaming it to `{}` to adhere to the naming convention.", + "Consider renaming it to `{}` to adhere to the naming convention.", mago_casing::to_camel_case(name) )), ); @@ -75,15 +81,18 @@ impl Walker> for FunctionRule { if !mago_casing::is_snake_case(name) { context.report( - Issue::new(context.level(), format!("function name `{}` should be in snake case", name)) - .with_annotations([ - Annotation::primary(function.name.span()), + Issue::new(context.level(), format!("Function name `{}` should be in snake case.", name)) + .with_annotation( + Annotation::primary(function.name.span()) + .with_message(format!("Function `{}` is declared here.`", name)), + ) + .with_annotation( Annotation::secondary(function.span()) - .with_message(format!("function `{}` is declared here", fqfn)), - ]) - .with_note(format!("the function name `{}` does not follow snake naming convention.", name)) + .with_message(format!("Function `{}` is defined here.", fqfn)), + ) + .with_note(format!("The function name `{}` does not follow snake naming convention.", name)) .with_help(format!( - "consider renaming it to `{}` to adhere to the naming convention.", + "Consider renaming it to `{}` to adhere to the naming convention.", mago_casing::to_snake_case(name) )), ); diff --git a/crates/linter/src/plugin/naming/rules/interface.rs b/crates/linter/src/plugin/naming/rules/interface.rs index e6b66b6..a91116c 100644 --- a/crates/linter/src/plugin/naming/rules/interface.rs +++ b/crates/linter/src/plugin/naming/rules/interface.rs @@ -28,15 +28,16 @@ impl<'a> Walker> for InterfaceRule { if !mago_casing::is_class_case(name) { issues.push( - Issue::new(context.level(), format!("interface name `{}` should be in class case", name)) + Issue::new(context.level(), format!("Interface name `{}` should be in class case.", name)) .with_annotations([ - Annotation::primary(interface.name.span()), + Annotation::primary(interface.name.span()) + .with_message(format!("Interface `{}` is declared here.", name)), Annotation::secondary(interface.span()) - .with_message(format!("interface `{}` is declared here", fqcn)), + .with_message(format!("Interface `{}` is defined here.", fqcn)), ]) - .with_note(format!("the interface name `{}` does not follow class naming convention.", name)) + .with_note(format!("The interface name `{}` does not follow class naming convention.", name)) .with_help(format!( - "consider renaming it to `{}` to adhere to the naming convention.", + "Consider renaming it to `{}` to adhere to the naming convention.", mago_casing::to_class_case(name) )), ); @@ -44,15 +45,16 @@ impl<'a> Walker> for InterfaceRule { if context.option("psr").and_then(|o| o.as_bool()).unwrap_or(true) && !name.ends_with("Interface") { issues.push( - Issue::new(context.level(), format!("interface name `{}` should be suffixed with `Interface`", name)) + Issue::new(context.level(), format!("interface name `{}` should be suffixed with `Interface`.", name)) .with_annotations([ - Annotation::primary(interface.name.span()), + Annotation::primary(interface.name.span()) + .with_message(format!("Interface `{}` is declared here.", name)), Annotation::secondary(interface.span()) - .with_message(format!("interface `{}` is declared here", fqcn)), + .with_message(format!("Interface `{}` is defined here.", fqcn)), ]) - .with_note(format!("the interface name `{}` does not follow PSR naming convention.", name)) + .with_note(format!("The interface name `{}` does not follow PSR naming convention.", name)) .with_help(format!( - "consider renaming it to `{}Interface` to adhere to the naming convention.", + "Consider renaming it to `{}Interface` to adhere to the naming convention.", name )), ); diff --git a/crates/linter/src/plugin/naming/rules/trait.rs b/crates/linter/src/plugin/naming/rules/trait.rs index c73ea34..f0ab4e3 100644 --- a/crates/linter/src/plugin/naming/rules/trait.rs +++ b/crates/linter/src/plugin/naming/rules/trait.rs @@ -28,15 +28,16 @@ impl<'a> Walker> for TraitRule { if !mago_casing::is_class_case(name) { issues.push( - Issue::new(context.level(), format!("trait name `{}` should be in class case", name)) + Issue::new(context.level(), format!("Trait name `{}` should be in class case.", name)) .with_annotations([ - Annotation::primary(r#trait.name.span()), + Annotation::primary(r#trait.name.span()) + .with_message(format!("Trait `{}` is declared here.", name)), Annotation::secondary(r#trait.span()) - .with_message(format!("trait `{}` is declared here", fqcn)), + .with_message(format!("Trait `{}` is defined here.", fqcn)), ]) - .with_note(format!("the trait name `{}` does not follow class naming convention.", name)) + .with_note(format!("The trait name `{}` does not follow class naming convention.", name)) .with_help(format!( - "consider renaming it to `{}` to adhere to the naming convention.", + "Consider renaming it to `{}` to adhere to the naming convention.", mago_casing::to_class_case(name) )), ); @@ -44,14 +45,15 @@ impl<'a> Walker> for TraitRule { if context.option("psr").and_then(|o| o.as_bool()).unwrap_or(true) && !name.ends_with("Trait") { issues.push( - Issue::new(context.level(), format!("trait name `{}` should be suffixed with `Trait`", name)) + Issue::new(context.level(), format!("Trait name `{}` should be suffixed with `Trait`.", name)) .with_annotations([ - Annotation::primary(r#trait.name.span()), + Annotation::primary(r#trait.name.span()) + .with_message(format!("Trait `{}` is declared here.", name)), Annotation::secondary(r#trait.span()) - .with_message(format!("trait `{}` is declared here", fqcn)), + .with_message(format!("Trait `{}` is defined here.", fqcn)), ]) - .with_note(format!("the trait name `{}` does not follow PSR naming convention.", name)) - .with_help(format!("consider renaming it to `{}Trait` to adhere to the naming convention.", name)), + .with_note(format!("The trait name `{}` does not follow PSR naming convention.", name)) + .with_help(format!("Consider renaming it to `{}Trait` to adhere to the naming convention.", name)), ); } diff --git a/crates/linter/src/plugin/phpunit/rules/consistency/assertions_style.rs b/crates/linter/src/plugin/phpunit/rules/consistency/assertions_style.rs index 14fef12..d638c0c 100644 --- a/crates/linter/src/plugin/phpunit/rules/consistency/assertions_style.rs +++ b/crates/linter/src/plugin/phpunit/rules/consistency/assertions_style.rs @@ -81,13 +81,13 @@ impl<'a> Walker> for AssertionsStyleRule { _ => unreachable!(), }; - let issue = Issue::new(context.level(), "inconsistent assertions style") - .with_annotations([Annotation::primary(reference.span()).with_message(format!( - "assertion style mismatch: expected `{}` style but found `{}` style.", - desired_style, current_style - ))]) + let issue = Issue::new(context.level(), "Inconsistent assertions style.") + .with_annotation( + Annotation::primary(reference.span()) + .with_message(format!("This assertion uses the `{}` style.", current_syntax)), + ) .with_help(format!( - "use `{}` instead of `{}` to conform to the `{}` style.", + "Use `{}` instead of `{}` to conform to the `{}` style.", desired_syntax, current_syntax, desired_style, )); diff --git a/crates/linter/src/plugin/phpunit/rules/strictness/strict_assertions.rs b/crates/linter/src/plugin/phpunit/rules/strictness/strict_assertions.rs index b8f4c83..447702d 100644 --- a/crates/linter/src/plugin/phpunit/rules/strictness/strict_assertions.rs +++ b/crates/linter/src/plugin/phpunit/rules/strictness/strict_assertions.rs @@ -41,11 +41,13 @@ impl<'a> Walker> for StrictAssertionsRule { if NON_STRICT_ASSERTIONS.contains(&name) { let strict_name = name.replacen("Equals", "Same", 1); - let issue = Issue::new(context.level(), "use strict assertions in PHPUnit tests") - .with_annotations([Annotation::primary(reference.span()) - .with_message(format!("replace `{}` with `{}`", name, strict_name))]) + let issue = Issue::new(context.level(), "Use strict assertions in PHPUnit tests.") + .with_annotation( + Annotation::primary(reference.span()) + .with_message(format!("Non-strict assertion `{}` is used here.", name)), + ) .with_help(format!( - "replace `{}` with `{}` to enforce strict comparisons in your tests.", + "Replace `{}` with `{}` to enforce strict comparisons in your tests.", name, strict_name )); diff --git a/crates/linter/src/plugin/redundancy/rules/redundant_block.rs b/crates/linter/src/plugin/redundancy/rules/redundant_block.rs index 6806519..c1ec212 100644 --- a/crates/linter/src/plugin/redundancy/rules/redundant_block.rs +++ b/crates/linter/src/plugin/redundancy/rules/redundant_block.rs @@ -12,13 +12,11 @@ pub struct RedundantBlockRule; impl RedundantBlockRule { fn report(&self, block: &Block, context: &mut LintContext<'_>) { - let issue = Issue::new(context.level(), "redundant block") + let issue = Issue::new(context.level(), "Redundant block around statements") .with_annotations([ - Annotation::primary(block.left_brace), - Annotation::primary(block.right_brace), - Annotation::secondary(block.span()).with_message("statements do not need to be wrapped within a block"), + Annotation::primary(block.span()).with_message("Statements do not need to be wrapped within a block.") ]) - .with_help("remove the redundant outer braces"); + .with_help("Remove the block to simplify the code."); context.report_with_fix(issue, |plan| { plan.delete(block.left_brace.to_range(), SafetyClassification::Safe); diff --git a/crates/linter/src/plugin/redundancy/rules/redundant_closing_tag.rs b/crates/linter/src/plugin/redundancy/rules/redundant_closing_tag.rs index 2e534fa..1cdbe9d 100644 --- a/crates/linter/src/plugin/redundancy/rules/redundant_closing_tag.rs +++ b/crates/linter/src/plugin/redundancy/rules/redundant_closing_tag.rs @@ -17,9 +17,9 @@ impl RedudnantClosingTagRule { }; if let Statement::ClosingTag(closing_tag) = last_statement { - let issue = Issue::new(context.level(), "redundant closing tag") - .with_annotation(Annotation::primary(closing_tag.span())) - .with_help("remove the redundant closing tag."); + let issue = Issue::new(context.level(), "Redundant closing tag ( `?>` ).") + .with_annotation(Annotation::primary(closing_tag.span()).with_message("This closing tag is redundant.")) + .with_help("Remove the redundant closing tag ( `?>` )."); context .report_with_fix(issue, |plan| plan.delete(closing_tag.span().to_range(), SafetyClassification::Safe)); @@ -39,10 +39,14 @@ impl RedudnantClosingTagRule { return; }; - let issue = Issue::new(context.level(), "redundant closing tag") - .with_annotation(Annotation::primary(tag.span())) - .with_annotation(Annotation::secondary(inline.span()).with_message("trailing whitespaces")) - .with_help("remove the redundant closing tag."); + let issue = + Issue::new(context.level(), "Redundant closing tag ( `?>` ) followed by trailing whitespace.") + .with_annotation(Annotation::primary(tag.span()).with_message("This closing tag is redundant.")) + .with_annotation( + Annotation::secondary(inline.span()) + .with_message("This inline statement is contains only whitespace."), + ) + .with_help("Remove the redundant closing tag ( `?>` ) and trailing whitespace."); context.report_with_fix(issue, |plan| { plan.delete(inline.span().to_range(), SafetyClassification::Safe); diff --git a/crates/linter/src/plugin/redundancy/rules/redundant_continue.rs b/crates/linter/src/plugin/redundancy/rules/redundant_continue.rs index ad0c16d..ffc1b70 100644 --- a/crates/linter/src/plugin/redundancy/rules/redundant_continue.rs +++ b/crates/linter/src/plugin/redundancy/rules/redundant_continue.rs @@ -22,7 +22,7 @@ impl Rule for RedundantContinueRule { impl RedundantContinueRule { fn report(&self, r#continue: &Continue, r#loop: impl HasSpan, context: &mut LintContext<'_>) { - let issue = Issue::new(context.level(), "Redundant continue statement in loop body") + let issue = Issue::new(context.level(), "Redundant continue statement in loop body.") .with_annotations([ Annotation::primary(r#continue.span()).with_message( "This `continue` statement is redundant because it is the last statement in the loop body.", diff --git a/crates/linter/src/plugin/redundancy/rules/redundant_final_method_modifier.rs b/crates/linter/src/plugin/redundancy/rules/redundant_final_method_modifier.rs index 0415784..6a366dd 100644 --- a/crates/linter/src/plugin/redundancy/rules/redundant_final_method_modifier.rs +++ b/crates/linter/src/plugin/redundancy/rules/redundant_final_method_modifier.rs @@ -23,21 +23,21 @@ impl RedundantFinalMethodModifierRule { let message = if in_enum { format!( - "the `final` modifier on method `{}` is redundant in enum `{}` as enums cannot be extended.", + "The `final` modifier on method `{}` is redundant in enum `{}` as enums cannot be extended.", method_name, class_like_name ) } else { format!( - "the `final` modifier on method `{}` is redundant as the class `{}` is already final.", + "The `final` modifier on method `{}` is redundant as the class `{}` is already final.", method_name, class_like_name ) }; let issue = Issue::new(context.level(), message) .with_annotations([ - Annotation::primary(final_modifier.span()), + Annotation::primary(final_modifier.span()).with_message("This `final` modifier is redundant."), Annotation::secondary(class_like_span) - .with_message(format!("{} `{}` defined here", class_like_kind, class_like_fqcn)), + .with_message(format!("{} `{}` defined here.", class_like_kind, class_like_fqcn)), ]) .with_help("Remove the redundant `final` modifier."); diff --git a/crates/linter/src/plugin/redundancy/rules/redundant_if_statement.rs b/crates/linter/src/plugin/redundancy/rules/redundant_if_statement.rs index e724a9e..8f75ba4 100644 --- a/crates/linter/src/plugin/redundancy/rules/redundant_if_statement.rs +++ b/crates/linter/src/plugin/redundancy/rules/redundant_if_statement.rs @@ -34,14 +34,14 @@ impl<'a> Walker> for RedundantIfStatementRule { // block // block // block - let issue = Issue::new(context.level(), "unnecessary `if` statement") + let issue = Issue::new(context.level(), "Unnecessary `if` statement.") .with_annotations([ - Annotation::primary(r#if.condition.span()).with_message("this condition always evaluates to true"), + Annotation::primary(r#if.condition.span()).with_message("This condition always evaluates to true"), Annotation::secondary(r#if.span()), ]) - .with_note("this `if` statement's condition always evaluates to true.") - .with_note("the `if` statement can be removed, and its body can be executed unconditionally.") - .with_help("remove the unnecessary `if` statement and execute its body directly."); + .with_note("This `if` statement's condition always evaluates to true.") + .with_note("The `if` statement can be removed, and its body can be executed unconditionally.") + .with_help("Remove the unnecessary `if` statement and execute its body directly."); context.report_with_fix(issue, |plan| { plan.delete(r#if.r#if.span.join(r#if.right_parenthesis).to_range(), SafetyClassification::Safe); @@ -119,15 +119,15 @@ impl<'a> Walker> for RedundantIfStatementRule { // if ($expr2) { block2 } else { block3 } // block2 // block2 - let issue = Issue::new(context.level(), "unnecessary `if` statement") + let issue = Issue::new(context.level(), "Unnecessary `if` statement.") .with_annotations([ Annotation::primary(r#if.condition.span()) - .with_message("this condition always evaluates to false."), + .with_message("This condition always evaluates to false."), Annotation::secondary(r#if.span()), ]) - .with_note("this `if` statement's condition always evaluates to false.") - .with_note("the `if` statement can be removed, and its body can be skipped.") - .with_help("remove the unnecessary `if` statement and skip its body."); + .with_note("This `if` statement's condition always evaluates to false.") + .with_note("The `if` statement can be removed, and its body can be skipped.") + .with_help("Remove the unnecessary `if` statement and skip its body."); context.report_with_fix(issue, |plan| match &r#if.body { IfBody::Statement(if_statement_body) => { diff --git a/crates/linter/src/plugin/redundancy/rules/redundant_label.rs b/crates/linter/src/plugin/redundancy/rules/redundant_label.rs index e2fbb49..08c0c5f 100644 --- a/crates/linter/src/plugin/redundancy/rules/redundant_label.rs +++ b/crates/linter/src/plugin/redundancy/rules/redundant_label.rs @@ -44,10 +44,10 @@ impl<'a> Walker> for RedundantLabelRule { let label_name = context.interner.lookup(&label_id); - let issue = Issue::new(context.level(), format!("redundant label: `{}`", label_name)) - .with_annotation(Annotation::primary(label_span)) - .with_note("this label is declared but not used by any `goto` statement.") - .with_help("remove the redundant label."); + let issue = Issue::new(context.level(), format!("Redundant goto label `{}`.", label_name)) + .with_annotation(Annotation::primary(label_span).with_message("This label is declared but not used.")) + .with_note(format!("Label `{}` is declared but not used by any `goto` statement.", label_name)) + .with_help("Remove the redundant label."); context.report_with_fix(issue, |plan| plan.delete(label_span.to_range(), SafetyClassification::Safe)); } diff --git a/crates/linter/src/plugin/redundancy/rules/redundant_method_override.rs b/crates/linter/src/plugin/redundancy/rules/redundant_method_override.rs index 747eda4..2961a0d 100644 --- a/crates/linter/src/plugin/redundancy/rules/redundant_method_override.rs +++ b/crates/linter/src/plugin/redundancy/rules/redundant_method_override.rs @@ -51,16 +51,16 @@ impl<'a> Walker> for RedundantMethodOverrideRule { }; if matches_method(&name, ¶meters, expression) { - let issue = Issue::new(context.level(), "redundant method override") + let issue = Issue::new(context.level(), "Redundant method override.") .with_annotation(Annotation::primary(method.span())) .with_annotation( Annotation::secondary(expression.span()) - .with_message("parent method is called with the same arguments"), + .with_message("Parent method is called with the same arguments."), ) .with_note( - "this method overrides a parent method but only calls the parent method with the same arguments.", + "This method overrides a parent method but only calls the parent method with the same arguments.", ) - .with_help("remove this redundant method override."); + .with_help("Remove this redundant method override."); context.report_with_fix(issue, |plan| { plan.delete(method.span().to_range(), SafetyClassification::PotentiallyUnsafe) diff --git a/crates/linter/src/plugin/redundancy/rules/redundant_noop.rs b/crates/linter/src/plugin/redundancy/rules/redundant_noop.rs index 89b36f7..7650a24 100644 --- a/crates/linter/src/plugin/redundancy/rules/redundant_noop.rs +++ b/crates/linter/src/plugin/redundancy/rules/redundant_noop.rs @@ -12,9 +12,9 @@ pub struct RedundantNoopRule; impl RedundantNoopRule { fn report(&self, noop: &Span, context: &mut LintContext<'_>) { - let issue = Issue::new(context.level(), "redundant noop statement") - .with_annotations([Annotation::primary(*noop)]) - .with_help("remove the redundant `;`"); + let issue = Issue::new(context.level(), "Redundant noop statement.") + .with_annotation(Annotation::primary(*noop).with_message("This is a redundant `noop` statement.")) + .with_help("Remove the redundant `;`"); context.report_with_fix(issue, |plan| plan.delete(noop.to_range(), SafetyClassification::Safe)); } diff --git a/crates/linter/src/plugin/redundancy/rules/redundant_parentheses.rs b/crates/linter/src/plugin/redundancy/rules/redundant_parentheses.rs index 027e68a..dcd7a74 100644 --- a/crates/linter/src/plugin/redundancy/rules/redundant_parentheses.rs +++ b/crates/linter/src/plugin/redundancy/rules/redundant_parentheses.rs @@ -12,14 +12,12 @@ pub struct RedundantParenthesesRule; impl RedundantParenthesesRule { fn report(&self, parenthesized: &Parenthesized, context: &mut LintContext<'_>) { - let issue = Issue::new(context.level(), "redundant parentheses") - .with_annotations([ - Annotation::primary(parenthesized.left_parenthesis), - Annotation::primary(parenthesized.right_parenthesis), - Annotation::secondary(parenthesized.expression.span()) - .with_message("expression does not need to be parenthesized"), - ]) - .with_help("remove the redundant inner parentheses"); + let issue = Issue::new(context.level(), "Redundant parentheses around expression.") + .with_annotation( + Annotation::primary(parenthesized.expression.span()) + .with_message("expression does not need to be parenthesized."), + ) + .with_help("Remove the redundant inner parentheses."); context.report_with_fix(issue, |plan| { plan.delete(parenthesized.left_parenthesis.to_range(), SafetyClassification::Safe); diff --git a/crates/linter/src/plugin/redundancy/rules/redundant_string_concat.rs b/crates/linter/src/plugin/redundancy/rules/redundant_string_concat.rs index b5555e5..7d98001 100644 --- a/crates/linter/src/plugin/redundancy/rules/redundant_string_concat.rs +++ b/crates/linter/src/plugin/redundancy/rules/redundant_string_concat.rs @@ -44,7 +44,6 @@ impl<'a> Walker> for RedundantStringConcatRule { } let dangerous = matches!(context.interner.lookup(&right.value)[1..].as_bytes(), [b'{', ..]); - if dangerous { // $a = "\u" . "{1F418}"; // $b = "\u{1F418}"; @@ -52,12 +51,12 @@ impl<'a> Walker> for RedundantStringConcatRule { return; } - let issue = Issue::new(context.level(), "string concatenation can be simplified") - .with_help("consider combining these strings into a single string") + let issue = Issue::new(context.level(), "String concatenation can be simplified.") + .with_help("Consider combining these strings into a single string.") .with_annotations(vec![ - Annotation::primary(operator.span()), - Annotation::secondary(left.span()), - Annotation::secondary(right.span()), + Annotation::primary(operator.span()).with_message("Redundant string concatenation."), + Annotation::secondary(left.span()).with_message("Left string"), + Annotation::secondary(right.span()).with_message("Right string"), ]); context.report_with_fix(issue, |plan| { diff --git a/crates/linter/src/plugin/safety/rules/no_error_control_operator.rs b/crates/linter/src/plugin/safety/rules/no_error_control_operator.rs index 1c099ee..d8901f3 100644 --- a/crates/linter/src/plugin/safety/rules/no_error_control_operator.rs +++ b/crates/linter/src/plugin/safety/rules/no_error_control_operator.rs @@ -23,11 +23,16 @@ impl Rule for NoErrorControlOperatorRule { impl<'a> Walker> for NoErrorControlOperatorRule { fn walk_in_unary_prefix<'ast>(&self, unary_prefix: &'ast UnaryPrefix, context: &mut LintContext<'a>) { if let UnaryPrefixOperator::ErrorControl(_) = unary_prefix.operator { - let issue = Issue::new(context.level(), "unsafe use of error control operator") - .with_annotation(Annotation::primary(unary_prefix.operator.span())) - .with_annotation(Annotation::secondary(unary_prefix.operand.span())) - .with_note("error control operator hide potential errors and make debugging more difficult.") - .with_help("remove the `@` and use `set_error_handler` to handle errors instead."); + let issue = Issue::new(context.level(), "Unsafe use of error control operator `@`.") + .with_annotation( + Annotation::primary(unary_prefix.operator.span()).with_message("This operator suppresses errors."), + ) + .with_annotation( + Annotation::secondary(unary_prefix.operand.span()) + .with_message("This expression is being suppressed."), + ) + .with_note("Error control operator hide potential errors and make debugging more difficult.") + .with_help("Remove the `@` and use `set_error_handler` to handle errors instead."); context.report_with_fix(issue, |plan| { plan.delete(unary_prefix.operator.span().to_range(), SafetyClassification::Safe) diff --git a/crates/linter/src/plugin/safety/rules/no_eval.rs b/crates/linter/src/plugin/safety/rules/no_eval.rs index 1c20b17..bc9da1d 100644 --- a/crates/linter/src/plugin/safety/rules/no_eval.rs +++ b/crates/linter/src/plugin/safety/rules/no_eval.rs @@ -21,13 +21,13 @@ impl Rule for NoEvalRule { impl<'a> Walker> for NoEvalRule { fn walk_in_eval_construct<'ast>(&self, eval_construct: &'ast EvalConstruct, context: &mut LintContext<'a>) { - let issue = Issue::new(context.level(), "unsafe use of `eval`") - .with_annotation(Annotation::primary(eval_construct.eval.span)) - .with_annotation(Annotation::secondary(eval_construct.value.span())) - .with_note("the `eval` construct executes arbitrary code, which can be a major security risk if not used carefully.") - .with_note("it can potentially lead to remote code execution vulnerabilities if the evaluated code is not properly sanitized.") - .with_note("consider using safer alternatives whenever possible.") - .with_help("avoid using `eval` unless absolutely necessary, and ensure that any dynamically generated code is properly validated and sanitized before execution.") + let issue = Issue::new(context.level(), "Unsafe use of `eval` construct.") + .with_annotation(Annotation::primary(eval_construct.eval.span).with_message("this `eval` construct is unsafe.")) + .with_annotation(Annotation::secondary(eval_construct.value.span()).with_message("the evaluated code is here.")) + .with_note("The `eval` construct executes arbitrary code, which can be a major security risk if not used carefully.") + .with_note("It can potentially lead to remote code execution vulnerabilities if the evaluated code is not properly sanitized.") + .with_note("Consider using safer alternatives whenever possible.") + .with_help("Avoid using `eval` unless absolutely necessary, and ensure that any dynamically generated code is properly validated and sanitized before execution.") ; context.report(issue); diff --git a/crates/linter/src/plugin/safety/rules/no_ffi.rs b/crates/linter/src/plugin/safety/rules/no_ffi.rs index 5a5878a..80db517 100644 --- a/crates/linter/src/plugin/safety/rules/no_ffi.rs +++ b/crates/linter/src/plugin/safety/rules/no_ffi.rs @@ -18,13 +18,13 @@ impl NoFFIRule { if FFI_CLASSES.iter().any(|ffi| ffi.eq_ignore_ascii_case(class_name)) { let mut issue = Issue::new( context.level(), - format!("potentionally unsafe use of FFI class `{}`", class_name), + format!("Potentionally unsafe use of FFI class `{}`.", class_name), ) .with_annotation(Annotation::primary(identifier.span())) .with_note("FFI (Foreign Function Interface) allows interaction with code written in other languages.") - .with_note("this can introduce potential security risks and stability issues if not handled carefully.") - .with_note("make sure you understand the implications and potential vulnerabilities before using FFI in production.") - .with_help("if possible, consider using alternative solutions within PHP to avoid relying on FFI."); + .with_note("This can introduce potential security risks and stability issues if not handled carefully.") + .with_note("Make sure you understand the implications and potential vulnerabilities before using FFI in production.") + .with_help("If possible, consider using alternative solutions within PHP to avoid relying on FFI."); if let Some(node) = node { issue = issue.with_annotation(Annotation::secondary(node.span())); diff --git a/crates/linter/src/plugin/safety/rules/no_global.rs b/crates/linter/src/plugin/safety/rules/no_global.rs index 59d17d9..4ce59dc 100644 --- a/crates/linter/src/plugin/safety/rules/no_global.rs +++ b/crates/linter/src/plugin/safety/rules/no_global.rs @@ -23,15 +23,15 @@ impl Rule for NoGlobalRule { impl<'a> Walker> for NoGlobalRule { fn walk_in_global<'ast>(&self, global: &'ast Global, context: &mut LintContext<'a>) { - let mut issue = Issue::new(context.level(), "unsafe use of `global`") - .with_annotation(Annotation::primary(global.global.span)) - .with_note("the `global` keyword introduces global state into your function, making it harder to reason about and test.") - .with_note("it can also lead to unexpected behavior and make your code more prone to errors.") - .with_note("consider using dependency injection or other techniques to manage state and avoid relying on global variables.") - .with_help("refactor your code to avoid using the `global` keyword."); + let mut issue = Issue::new(context.level(), "Unsafe use of `global` keyword.") + .with_annotation(Annotation::primary(global.global.span).with_message("This `global` keyword is used here.")) + .with_note("The `global` keyword introduces global state into your function, making it harder to reason about and test.") + .with_note("It can also lead to unexpected behavior and make your code more prone to errors.") + .with_note("Consider using dependency injection or other techniques to manage state and avoid relying on global variables.") + .with_help("Refactor your code to avoid using the `global` keyword."); for variable in global.variables.iter() { - issue = issue.with_annotation(Annotation::secondary(variable.span())) + issue = issue.with_annotation(Annotation::secondary(variable.span())); } context.report(issue); @@ -43,12 +43,12 @@ impl<'a> Walker> for NoGlobalRule { return; } - let issue = Issue::new(context.level(), "unsafe use of `$GLOBAL` variable") - .with_annotation(Annotation::primary(direct_variable.span)) - .with_note("accessing the `$GLOBALS` array directly can lead to similar issues as using the `global` keyword.") - .with_note("it can make your code harder to understand, test, and maintain due to the implicit global state.") - .with_note("consider using dependency injection or other techniques to manage state and avoid relying on global variables.") - .with_help("refactor your code to avoid using the `$GLOBALS` variable directly."); + let issue = Issue::new(context.level(), "Unsafe use of `$GLOBAL` variable.") + .with_annotation(Annotation::primary(direct_variable.span).with_message("The `$GLOBALS` variable is used here.")) + .with_note("Accessing the `$GLOBALS` array directly can lead to similar issues as using the `global` keyword.") + .with_note("It can make your code harder to understand, test, and maintain due to the implicit global state.") + .with_note("Consider using dependency injection or other techniques to manage state and avoid relying on global variables.") + .with_help("Refactor your code to avoid using the `$GLOBALS` variable directly."); context.report(issue); } diff --git a/crates/linter/src/plugin/safety/rules/no_request_variable.rs b/crates/linter/src/plugin/safety/rules/no_request_variable.rs index c1999d6..c2ca243 100644 --- a/crates/linter/src/plugin/safety/rules/no_request_variable.rs +++ b/crates/linter/src/plugin/safety/rules/no_request_variable.rs @@ -27,8 +27,10 @@ impl<'a> Walker> for NoRequestVariableRule { return; } - let issue = Issue::new(context.level(), "unsafe use of `$_REQUEST` variable") - .with_annotation(Annotation::primary(direct_variable.span)) + let issue = Issue::new(context.level(), "Unsafe use of `$_REQUEST` variable.") + .with_annotation( + Annotation::primary(direct_variable.span).with_message("The `$_REQUEST` variable is used here."), + ) .with_help("use `$_GET`, `$_POST`, or `$_COOKIE` instead for better clarity."); context.report(issue); diff --git a/crates/linter/src/plugin/safety/rules/no_shell_execute_string.rs b/crates/linter/src/plugin/safety/rules/no_shell_execute_string.rs index 1ee236f..c9a0f65 100644 --- a/crates/linter/src/plugin/safety/rules/no_shell_execute_string.rs +++ b/crates/linter/src/plugin/safety/rules/no_shell_execute_string.rs @@ -35,17 +35,19 @@ impl<'a> Walker> for NoShellExecuteStringRule { } let issue = if is_interpolated { - Issue::new(context.level(), "unsafe use of interpolated shell execute string") - .with_annotation(Annotation::primary(shell_execute_string.span())) - .with_note("interpolating shell execute strings (`...`) is a potential security vulnerability, as it allows executing arbitrary shell commands.") + Issue::new(context.level(), "Unsafe use of interpolated shell execute string.") + .with_annotation(Annotation::primary(shell_execute_string.span()).with_message("This shell execute string is interpolated.")) + .with_note("Interpolating shell execute strings (`...`) is a potential security vulnerability, as it allows executing arbitrary shell commands.") .with_help( - "consider using `shell_exec()` along with `escapeshellarg()` or `escapeshellcmd()` to escape arguments instead." + "Consider using `shell_exec()` along with `escapeshellarg()` or `escapeshellcmd()` to escape arguments instead." ) } else { - Issue::new(context.level(), "potentilly unsafe use of shell execute string") - .with_annotation(Annotation::primary(shell_execute_string.span())) - .with_note("shell execute strings (`...`) can often be replaced with safer alternatives.") - .with_help("consider using `shell_exec()` instead.") + Issue::new(context.level(), "Potentilly unsafe use of shell execute string.") + .with_annotation( + Annotation::primary(shell_execute_string.span()).with_message("Shell execute string used here."), + ) + .with_note("Shell execute strings (`...`) can often be replaced with safer alternatives.") + .with_help("Consider using `shell_exec()` instead.") }; context.report(issue); diff --git a/crates/linter/src/plugin/strictness/rules/missing_assert_description.rs b/crates/linter/src/plugin/strictness/rules/missing_assert_description.rs index 2e13f01..dd2f5d8 100644 --- a/crates/linter/src/plugin/strictness/rules/missing_assert_description.rs +++ b/crates/linter/src/plugin/strictness/rules/missing_assert_description.rs @@ -37,9 +37,9 @@ impl<'a> Walker> for MissingAssertDescriptionRule { } if function_call.arguments.arguments.get(1).is_none() { - let issue = Issue::new(context.level(), "missing description in assert") - .with_annotation(Annotation::primary(function_call.span())) - .with_help("add a description to the assert function to make it easier to understand the purpose of the assertion."); + let issue = Issue::new(context.level(), "Missing description in assert function.") + .with_annotation(Annotation::primary(function_call.span()).with_message("`assert` function is called here.")) + .with_help("Add a description to the assert function to make it easier to understand the purpose of the assertion."); context.report(issue); } diff --git a/crates/linter/src/plugin/strictness/rules/no_assignment_in_condition.rs b/crates/linter/src/plugin/strictness/rules/no_assignment_in_condition.rs index 784654d..4347b4c 100644 --- a/crates/linter/src/plugin/strictness/rules/no_assignment_in_condition.rs +++ b/crates/linter/src/plugin/strictness/rules/no_assignment_in_condition.rs @@ -12,13 +12,13 @@ pub struct NoAssignmentInConditionRule; impl NoAssignmentInConditionRule { fn report<'ast>(&self, condition: &'ast Expression, assignment: &'ast Assignment, context: &mut LintContext) { - let mut issue = Issue::new(context.level(), "avoid assignments in conditions") - .with_annotation(Annotation::primary(assignment.span())) - .with_annotation(Annotation::secondary(condition.span())) - .with_note("assigning a value within a condition can lead to unexpected behavior and make the code harder to read and understand."); + let mut issue = Issue::new(context.level(), "Avoid assignments in conditions.") + .with_annotation(Annotation::primary(assignment.span()).with_message("This is an assignment.")) + .with_annotation(Annotation::secondary(condition.span()).with_message("This is the condition.")) + .with_note("Assigning a value within a condition can lead to unexpected behavior and make the code harder to read and understand."); if matches!(&assignment.operator, AssignmentOperator::Assign(_)) { - issue = issue.with_note("it's easy to confuse assignment (`=`) with comparison (`==`) in this context. ensure you're using the correct operator."); + issue = issue.with_note("It's easy to confuse assignment (`=`) with comparison (`==`) in this context. ensure you're using the correct operator."); } context.report(issue); diff --git a/crates/linter/src/plugin/strictness/rules/require_constant_type.rs b/crates/linter/src/plugin/strictness/rules/require_constant_type.rs index f110757..fa527d3 100644 --- a/crates/linter/src/plugin/strictness/rules/require_constant_type.rs +++ b/crates/linter/src/plugin/strictness/rules/require_constant_type.rs @@ -31,28 +31,18 @@ impl<'a> Walker> for RequireConstantTypeRule { return; } - let (class_like_kind, class_like_name, class_like_fqcn, class_like_span) = - context.get_class_like_details(class_like_constant); - - for item in class_like_constant.items.iter() { - let constant_name = context.lookup(&item.name.value); - - context.report( - Issue::new( - context.level(), - format!( - "{} constant `{}::{}` is missing a type hint", - class_like_kind, class_like_name, constant_name - ), + let item = class_like_constant.first_item(); + + let constant_name = context.lookup(&item.name.value); + + context.report( + Issue::new(context.level(), format!("Class constant `{}` is missing a type hint.", constant_name)) + .with_annotation( + Annotation::primary(class_like_constant.span()) + .with_message(format!("Class constant `{}` is defined here.", constant_name)), ) - .with_annotations([ - Annotation::primary(class_like_constant.span()), - Annotation::secondary(class_like_span) - .with_message(format!("{} `{}` declared here", class_like_kind, class_like_fqcn)), - ]) - .with_note("adding a type hint to constants improves code readability and helps prevent type errors.") - .with_help(format!("consider specifying a type hint for `{}::{}`.", class_like_name, constant_name)), - ); - } + .with_note("Adding a type hint to constants improves code readability and helps prevent type errors.") + .with_help(format!("Consider specifying a type hint for `{}`.", constant_name)), + ); } } diff --git a/crates/linter/src/plugin/strictness/rules/require_identity_comparison.rs b/crates/linter/src/plugin/strictness/rules/require_identity_comparison.rs index 3f06001..c452b05 100644 --- a/crates/linter/src/plugin/strictness/rules/require_identity_comparison.rs +++ b/crates/linter/src/plugin/strictness/rules/require_identity_comparison.rs @@ -1,7 +1,6 @@ use mago_ast::ast::*; use mago_fixer::SafetyClassification; use mago_reporting::*; -use mago_span::*; use mago_walker::Walker; use crate::context::LintContext; @@ -25,36 +24,46 @@ impl Rule for RequireIdentityComparisonRule { impl<'a> Walker> for RequireIdentityComparisonRule { fn walk_in_binary<'ast>(&self, binary: &'ast Binary, context: &mut LintContext<'a>) { match &binary.operator { + // `==` -> `===` BinaryOperator::Equal(span) => { let issue = - Issue::new(context.level(), "use identity comparison `===` instead of equality comparison `==`") - .with_annotations([ - Annotation::primary(*span), - Annotation::secondary(binary.lhs.span()), - Annotation::secondary(binary.rhs.span()), - ]) + Issue::new(context.level(), "Use identity comparison `===` instead of equality comparison `==`.") + .with_annotation(Annotation::primary(*span).with_message("Equality operator is used here.")) .with_note( - "identity comparison `===` checks for both value and type equality, \ - while equality comparison `==` performs type coercion, which can lead to unexpected results", + "Identity comparison `===` checks for both value and type equality, \ + while equality comparison `==` performs type coercion, which can lead to unexpected results.", ) - .with_help("use `===` to ensure both value and type are equal"); + .with_help("Use `===` to ensure both value and type are equal."); context .report_with_fix(issue, |plan| plan.replace(span.to_range(), "===", SafetyClassification::Unsafe)); } + // `!=` -> `!==` BinaryOperator::NotEqual(span) => { let issue = - Issue::new(context.level(), "use identity inequality `!==` instead of inequality comparison `!=`") - .with_annotations([ - Annotation::primary(*span), - Annotation::secondary(binary.lhs.span()), - Annotation::secondary(binary.rhs.span()), - ]) + Issue::new(context.level(), "Use identity inequality `!==` instead of inequality comparison `!=`.") + .with_annotation(Annotation::primary(*span).with_message("Inequality operator is used here.")) .with_note( - "identity inequality `!==` checks for both value and type inequality, \ - while inequality comparison `!=` performs type coercion, which can lead to unexpected results", + "Identity inequality `!==` checks for both value and type inequality, \ + while inequality comparison `!=` performs type coercion, which can lead to unexpected results.", ) - .with_help("use `!==` to ensure both value and type are different"); + .with_help("Use `!==` to ensure both value and type are different."); + + context + .report_with_fix(issue, |plan| plan.replace(span.to_range(), "!==", SafetyClassification::Unsafe)); + } + // `<>` -> `!==` + BinaryOperator::AngledNotEqual(span) => { + let issue = Issue::new( + context.level(), + "Use identity inequality `!==` instead of angled inequality comparison `<>`.", + ) + .with_annotation(Annotation::primary(*span).with_message("Angled inequality operator is used here.")) + .with_note( + "Identity inequality `!==` checks for both value and type inequality, \ + while angled inequality comparison `<>` performs type coercion, which can lead to unexpected results.", + ) + .with_help("Use `!==` to ensure both value and type are different."); context .report_with_fix(issue, |plan| plan.replace(span.to_range(), "!==", SafetyClassification::Unsafe)); diff --git a/crates/linter/src/plugin/strictness/rules/require_parameter_type.rs b/crates/linter/src/plugin/strictness/rules/require_parameter_type.rs index 0143f50..07ea968 100644 --- a/crates/linter/src/plugin/strictness/rules/require_parameter_type.rs +++ b/crates/linter/src/plugin/strictness/rules/require_parameter_type.rs @@ -34,10 +34,13 @@ impl<'a> Walker> for RequireParameterTypeRule { let parameter_name = context.lookup(&function_like_parameter.variable.name); context.report( - Issue::new(context.level(), format!("parameter `{}` is missing a type hint", parameter_name)) - .with_annotation(Annotation::primary(function_like_parameter.span())) - .with_note("type hints improve code readability and help prevent type-related errors.") - .with_help(format!("consider adding a type hint to parameter `{}`.", parameter_name)), + Issue::new(context.level(), format!("Parameter `{}` is missing a type hint.", parameter_name)) + .with_annotation( + Annotation::primary(function_like_parameter.span()) + .with_message(format!("Parameter `{}` is declared here", parameter_name)), + ) + .with_note("Type hints improve code readability and help prevent type-related errors.") + .with_help(format!("Consider adding a type hint to parameter `{}`.", parameter_name)), ); } } diff --git a/crates/linter/src/plugin/strictness/rules/require_property_type.rs b/crates/linter/src/plugin/strictness/rules/require_property_type.rs index 7ec11f3..0647217 100644 --- a/crates/linter/src/plugin/strictness/rules/require_property_type.rs +++ b/crates/linter/src/plugin/strictness/rules/require_property_type.rs @@ -27,26 +27,15 @@ impl<'a> Walker> for RequirePropertyTypeRule { return; } - let (class_like_kind, class_like_name, class_like_fqcn, class_like_span) = - context.get_class_like_details(property); - - let variable = context.lookup(&property.first_variable().name); + let name = context.lookup(&property.first_variable().name); context.report( - Issue::new( - context.level(), - format!("{} property `{}::{}` is missing a type hint", class_like_kind, class_like_name, variable), - ) - .with_annotation(Annotation::primary(property.span())) - .with_annotation( - Annotation::secondary(class_like_span) - .with_message(format!("{} `{}` declared here", class_like_kind, class_like_fqcn)), - ) - .with_note(format!( - "adding a type hint to {} properties improves code readability and helps prevent type errors.", - class_like_kind - )) - .with_help(format!("consider specifying a type hint for `{}::{}`.", class_like_name, variable)), + Issue::new(context.level(), format!("Property `{}` is missing a type hint.", name)) + .with_annotation( + Annotation::primary(property.span()).with_message(format!("Property `{}` is declared here.", name)), + ) + .with_note("Adding a type hint to properties improves code readability and helps prevent type errors.") + .with_help(format!("Consider specifying a type hint for `{}`.", name)), ); } } diff --git a/crates/linter/src/plugin/strictness/rules/require_return_type.rs b/crates/linter/src/plugin/strictness/rules/require_return_type.rs index edc277a..f3f2be6 100644 --- a/crates/linter/src/plugin/strictness/rules/require_return_type.rs +++ b/crates/linter/src/plugin/strictness/rules/require_return_type.rs @@ -31,14 +31,13 @@ impl<'a> Walker> for RequireReturnTypeRule { let function_fqn = context.lookup_name(&function.name); context.report( - Issue::new(context.level(), format!("function `{}` is missing a return type hint", function_name)) - .with_annotation(Annotation::primary(function.name.span())) + Issue::new(context.level(), format!("Function `{}` is missing a return type hint.", function_name)) .with_annotation( - Annotation::secondary(function.span()) - .with_message(format!("function `{}` defined here", function_fqn)), + Annotation::primary(function.span()) + .with_message(format!("Function `{}` defined here.", function_fqn)), ) - .with_note("type hints improve code readability and help prevent type-related errors.") - .with_help(format!("consider adding a return type hint to function `{}`.", function_name)), + .with_note("Type hints improve code readability and help prevent type-related errors.") + .with_help(format!("Consider adding a return type hint to function `{}`.", function_name)), ); } @@ -48,11 +47,10 @@ impl<'a> Walker> for RequireReturnTypeRule { } context.report( - Issue::new(context.level(), "closure is missing a return type hint") - .with_annotation(Annotation::primary(closure.function.span())) - .with_annotation(Annotation::secondary(closure.span()).with_message("closure defined here")) - .with_note("type hints improve code readability and help prevent type-related errors.") - .with_help("consider adding a return type hint to the closure."), + Issue::new(context.level(), "Closure is missing a return type hint") + .with_annotation(Annotation::primary(closure.span()).with_message("Closure defined here.")) + .with_note("Type hints improve code readability and help prevent type-related errors.") + .with_help("Consider adding a return type hint to the closure."), ); } @@ -62,13 +60,12 @@ impl<'a> Walker> for RequireReturnTypeRule { } context.report( - Issue::new(context.level(), "arrow function is missing a return type hint") - .with_annotation(Annotation::primary(arrow_function.r#fn.span())) + Issue::new(context.level(), "Arrow function is missing a return type hint.") .with_annotation( - Annotation::secondary(arrow_function.span()).with_message("arrow function defined here"), + Annotation::secondary(arrow_function.span()).with_message("Arrow function defined here."), ) - .with_note("type hints improve code readability and help prevent type-related errors.") - .with_help("consider adding a return type hint to the arrow function."), + .with_note("Type hints improve code readability and help prevent type-related errors.") + .with_help("Consider adding a return type hint to the arrow function."), ); } @@ -83,26 +80,13 @@ impl<'a> Walker> for RequireReturnTypeRule { return; } - let (class_like_kind, class_like_name, class_like_fqcn, class_like_span) = - context.get_class_like_details(method); - context.report( - Issue::new( - context.level(), - format!( - "{} method `{}::{}` is missing a return type hint", - class_like_kind, class_like_name, method_name - ), - ) - .with_annotation(Annotation::primary(method.name.span())) - .with_annotations([ - Annotation::secondary(method.span()) - .with_message(format!("{} method `{}` defined here", class_like_kind, method_name)), - Annotation::secondary(class_like_span) - .with_message(format!("{} `{}` defined here", class_like_kind, class_like_fqcn)), - ]) - .with_note("type hints improve code readability and help prevent type-related errors.") - .with_help(format!("consider adding a return type hint to {} method `{}`.", class_like_kind, method_name)), + Issue::new(context.level(), format!("Method `{}` is missing a return type hint.", method_name)) + .with_annotation( + Annotation::secondary(method.span()).with_message(format!("Method `{}` defined here", method_name)), + ) + .with_note("Type hints improve code readability and help prevent type-related errors.") + .with_help(format!("Consider adding a return type hint to method `{}`.", method_name)), ); } } diff --git a/crates/linter/src/plugin/strictness/rules/require_strict_types.rs b/crates/linter/src/plugin/strictness/rules/require_strict_types.rs index 89982b6..824e284 100644 --- a/crates/linter/src/plugin/strictness/rules/require_strict_types.rs +++ b/crates/linter/src/plugin/strictness/rules/require_strict_types.rs @@ -55,14 +55,13 @@ impl<'a> Walker> for RequireStrictTypesRule { if disabled && !context.option("allow-disabling").and_then(|o| o.as_bool()).unwrap_or(false) { context.report( - Issue::new(context.level(), "`strict_types` is disabled") - .with_annotation(Annotation::primary(item.value.span())) + Issue::new(context.level(), "The `strict_types` directive is disabled.") .with_annotation( - Annotation::secondary(item.name.span()) - .with_message("`strict_types` directive here"), + Annotation::primary(item.span()) + .with_message("The `strict_types` is disabled here."), ) - .with_note("disabling `strict_types` can lead to type safety issues.") - .with_help("consider setting `strict_types` to `1` to enforce strict typing."), + .with_note("Disabling `strict_types` can lead to type safety issues.") + .with_help("Consider setting `strict_types` to `1` to enforce strict typing."), ); } } @@ -80,11 +79,11 @@ impl<'a> Walker> for RequireStrictTypesRule { context.report( Issue::new( context.level(), - "missing `declare(strict_types=1);` statement at the beginning of the file", + "Missing `declare(strict_types=1);` statement at the beginning of the file", ) .with_annotation(Annotation::primary(program.span())) - .with_note("`strict_types` enforces strict type checking, which can prevent subtle bugs.") - .with_help("add `declare(strict_types=1);` at the top of your file."), + .with_note("The `strict_types` directive enforces strict type checking, which can prevent subtle bugs.") + .with_help("Add `declare(strict_types=1);` at the top of your file."), ); } } diff --git a/crates/linter/src/plugin/symfony/rules/quality/interface_should_be_used.rs b/crates/linter/src/plugin/symfony/rules/quality/interface_should_be_used.rs index 784df37..9ec9a08 100644 --- a/crates/linter/src/plugin/symfony/rules/quality/interface_should_be_used.rs +++ b/crates/linter/src/plugin/symfony/rules/quality/interface_should_be_used.rs @@ -31,9 +31,12 @@ impl<'a> Walker> for InterfaceShouldBeUsed { if fqcn == *implementation { let issue = Issue::new( context.level(), - format!("use the interface `{}` instead of the implementation `{}`", interface, implementation,), + format!("Use the interface `{}` instead of the implementation `{}`", interface, implementation,), ) - .with_annotation(Annotation::primary(identifier.span()).with_message("interface should be used")); + .with_annotation( + Annotation::primary(identifier.span()) + .with_message("This uses the implementation instead of the interface."), + ); context.report_with_fix(issue, |plan| { // the change is potentially unsafe because we don't diff --git a/crates/parser/src/error.rs b/crates/parser/src/error.rs index 66ffe05..c8e4d56 100644 --- a/crates/parser/src/error.rs +++ b/crates/parser/src/error.rs @@ -39,11 +39,11 @@ impl std::fmt::Display for ParseError { let expected = expected.iter().map(|kind| kind.to_string()).collect::>().join("`, `"); if expected.is_empty() { - "unexpected end of file".to_string() + "Unexpected end of file".to_string() } else if expected.len() == 1 { - format!("expected `{}` before end of file", expected) + format!("Expected `{}` before end of file", expected) } else { - format!("expected one of `{}` before end of file", expected) + format!("Expected one of `{}` before end of file", expected) } } ParseError::UnexpectedToken(expected, found, _) => { @@ -52,20 +52,20 @@ impl std::fmt::Display for ParseError { let found = found.to_string(); if expected.is_empty() { - format!("unexpected token `{}`", found) + format!("Unexpected token `{}`", found) } else if expected.len() == 1 { - format!("expected `{}`, found `{}`", expected, found) + format!("Expected `{}`, found `{}`", expected, found) } else { - format!("expected one of `{}`, found `{}`", expected, found) + format!("Expected one of `{}`, found `{}`", expected, found) } } ParseError::UnclosedLiteralString(kind, _) => match kind { - LiteralStringKind::SingleQuoted => "unclosed single-quoted string".to_string(), - LiteralStringKind::DoubleQuoted => "unclosed double-quoted string".to_string(), + LiteralStringKind::SingleQuoted => "Unclosed single-quoted string".to_string(), + LiteralStringKind::DoubleQuoted => "Unclosed double-quoted string".to_string(), }, }; - write!(f, "parse error: {}", message) + write!(f, "{}", message) } } @@ -88,6 +88,6 @@ impl From<&ParseError> for Issue { fn from(error: &ParseError) -> Self { let span = error.span(); - Issue::error(error.to_string()).with_annotation(Annotation::primary(span)) + Issue::error(error.to_string()).with_annotation(Annotation::primary(span).with_message("Invalid syntax.")) } } diff --git a/crates/semantics/src/walker.rs b/crates/semantics/src/walker.rs index 004ba65..b8e1f3e 100644 --- a/crates/semantics/src/walker.rs +++ b/crates/semantics/src/walker.rs @@ -39,6 +39,7 @@ use crate::context::Context; pub struct SemanticsWalker; impl SemanticsWalker { + #[inline] fn process_extends( &self, extends: &Extends, @@ -52,16 +53,17 @@ impl SemanticsWalker { if extension_limit && extends.types.len() > 1 { context.report( Issue::error(format!( - "{} `{}` can only extend one other tyoe, found {}", + "{} `{}` can only extend one other type, found {}.", class_like_kind, class_like_name, extends.types.len() )) - .with_annotation(Annotation::primary(extends.span())) + .with_annotation(Annotation::primary(extends.span()).with_message("Multiple extensions found here.")) .with_annotation( Annotation::secondary(class_like_span) - .with_message(format!("{} `{}` defined here", class_like_kind, class_like_fqcn)), - ), + .with_message(format!("{} `{}` declared here.", class_like_kind, class_like_fqcn)), + ) + .with_help("Remove the extra extensions to ensure only one type is extended."), ); } @@ -70,12 +72,18 @@ impl SemanticsWalker { if extended_fqcn.eq_ignore_ascii_case(class_like_fqcn) { context.report( - Issue::error(format!("{} `{}` cannot extend itself", class_like_kind, class_like_name)) - .with_annotation(Annotation::primary(extended_type.span())) + Issue::error(format!("{} `{}` cannot extend itself.", class_like_kind, class_like_name)) + .with_annotation( + Annotation::primary(extended_type.span()).with_message(format!( + "{} `{}` extends itself here.", + class_like_kind, class_like_name + )), + ) .with_annotation( Annotation::secondary(class_like_span) - .with_message(format!("{} `{}` defined here", class_like_kind, class_like_fqcn)), - ), + .with_message(format!("{} `{}` declared here.", class_like_kind, class_like_fqcn)), + ) + .with_help("Remove the self-referencing extension."), ); } } @@ -90,19 +98,26 @@ impl SemanticsWalker { { context.report( Issue::error(format!( - "{} `{}` cannot extend reserved keyword `{}`", + "{} `{}` cannot extend reserved keyword `{}`.", class_like_kind, class_like_name, extended_name )) - .with_annotation(Annotation::primary(extended_type.span())) + .with_annotation( + Annotation::primary(extended_type.span()).with_message("Extension uses a reserved keyword."), + ) .with_annotation( Annotation::secondary(class_like_span) - .with_message(format!("{} `{}` defined here", class_like_kind, class_like_name)), - ), + .with_message(format!("{} `{}` declared here.", class_like_kind, class_like_name)), + ) + .with_help(format!( + "Change the extended type to a valid identifier. `{}` is a reserved keyword.", + extended_name + )), ); } } } + #[inline] fn process_implements( &self, implements: &Implements, @@ -119,12 +134,16 @@ impl SemanticsWalker { if implemented_fqcn.eq_ignore_ascii_case(class_like_fqcn) { context.report( - Issue::error(format!("{} `{}` cannot implement itself", class_like_kind, class_like_name)) - .with_annotation(Annotation::primary(implemented_type.span())) + Issue::error(format!("{} `{}` cannot implement itself.", class_like_kind, class_like_name)) + .with_annotation(Annotation::primary(implemented_type.span()).with_message(format!( + "{} `{}` implements itself here.", + class_like_kind, class_like_name + ))) .with_annotation( Annotation::secondary(class_like_span) - .with_message(format!("{} `{}` defined here", class_like_kind, class_like_fqcn)), - ), + .with_message(format!("{} `{}` declared here.", class_like_kind, class_like_fqcn)), + ) + .with_help("Remove the self-referencing implementation."), ); } } @@ -140,19 +159,26 @@ impl SemanticsWalker { { context.report( Issue::error(format!( - "{} `{}` cannot implement reserved keyword `{}`", + "{} `{}` cannot implement reserved keyword `{}`.", class_like_kind, class_like_name, implemented_name )) - .with_annotation(Annotation::primary(implemented_type.span())) + .with_annotation( + Annotation::primary(implemented_type.span()).with_message("This is a reserved keyword."), + ) .with_annotation( Annotation::secondary(class_like_span) - .with_message(format!("{} `{}` defined here", class_like_kind, class_like_name)), - ), + .with_message(format!("{} `{}` declared here.", class_like_kind, class_like_name)), + ) + .with_help(format!( + "Replace `{}` with a valid identifier. Reserved keywords cannot be used as implemented types.", + implemented_name + )), ); } } } + #[inline] fn process_property( &self, property: &Property, @@ -179,17 +205,20 @@ impl SemanticsWalker { Modifier::Abstract(_) => { context.report( Issue::error(format!( - "property `{}:{}` cannot be declared abstract", + "Property `{}::{}` cannot be declared abstract", class_like_name, first_variable_name )) - .with_annotation(Annotation::primary(modifier.span())) + .with_annotation( + Annotation::primary(modifier.span()) + .with_message("`abstract` modifier cannot be used on properties"), + ) .with_annotation( Annotation::secondary(first_variable.span()) - .with_message(format!("property `{}`", first_variable_name)), + .with_message(format!("Property `{}` declared here.", first_variable_name)), ) .with_annotation( Annotation::secondary(class_like_span) - .with_message(format!("{} `{}` defined here", class_like_kind, class_like_fqcn)), + .with_message(format!("{} `{}` defined here.", class_like_kind, class_like_fqcn)), ), ); } @@ -197,17 +226,23 @@ impl SemanticsWalker { if let Some(last_readonly) = last_readonly { context.report( Issue::error(format!( - "readonly property `{}::{}` cannot be static", + "Readonly property `{}::{}` cannot be static.", class_like_name, first_variable_name )) - .with_annotation(Annotation::primary(last_readonly)) + .with_annotation( + Annotation::primary(modifier.span()) + .with_message("`static` modifier cannot be used on readonly properties."), + ) + .with_annotation( + Annotation::primary(last_readonly).with_message("Property is marked as readonly here."), + ) .with_annotation( Annotation::secondary(first_variable.span()) - .with_message(format!("property `{}`", first_variable_name)), + .with_message(format!("Property `{}` declared here.", first_variable_name)), ) .with_annotation( Annotation::secondary(class_like_span) - .with_message(format!("{} `{}` defined here", class_like_kind, class_like_fqcn)), + .with_message(format!("{} `{}` defined here.", class_like_kind, class_like_fqcn)), ), ); } @@ -215,20 +250,22 @@ impl SemanticsWalker { if let Some(last_static) = last_static { context.report( Issue::error(format!( - "property `{}::{}` has multiple `static` modifiers", + "Property `{}::{}` has multiple `static` modifiers.", class_like_name, first_variable_name )) - .with_annotation(Annotation::primary(modifier.span())) .with_annotation( - Annotation::secondary(last_static).with_message("previous `static` modifier"), + Annotation::primary(modifier.span()).with_message("Duplicate `static` modifier."), + ) + .with_annotation( + Annotation::secondary(last_static).with_message("Previous `static` modifier."), ) .with_annotation( Annotation::secondary(first_variable.span()) - .with_message(format!("property `{}`", first_variable_name)), + .with_message(format!("Property `{}` declared here.", first_variable_name)), ) .with_annotation( Annotation::secondary(class_like_span) - .with_message(format!("{} `{}` defined here", class_like_kind, class_like_fqcn)), + .with_message(format!("{} `{}` defined here.", class_like_kind, class_like_fqcn)), ), ); } @@ -236,18 +273,22 @@ impl SemanticsWalker { if let Some(last_visibility) = last_write_visibility { context.report( Issue::error(format!( - "static property `{}::{}` cannot have a write visibility modifier", + "static property `{}::{}` cannot have a write visibility modifier.", class_like_name, first_variable_name )) - .with_annotation(Annotation::primary(last_visibility)) - .with_annotation(Annotation::primary(modifier.span()).with_message("`static` modifier")) + .with_annotation( + Annotation::primary(modifier.span()).with_message("Duplicate visibility modifier."), + ) + .with_annotation( + Annotation::secondary(last_visibility).with_message("Previous visibility modifier."), + ) .with_annotation( Annotation::secondary(first_variable.span()) - .with_message(format!("property `{}`", first_variable_name)), + .with_message(format!("Property `{}` declared here.", first_variable_name)), ) .with_annotation( Annotation::secondary(class_like_span) - .with_message(format!("{} `{}` defined here", class_like_kind, class_like_fqcn)), + .with_message(format!("{} `{}` defined here.", class_like_kind, class_like_fqcn)), ), ); } @@ -258,18 +299,23 @@ impl SemanticsWalker { if let Some(last_static) = last_static { context.report( Issue::error(format!( - "static property `{}::{}` cannot be readonly", + "Static property `{}::{}` cannot be readonly.", class_like_name, first_variable_name )) - .with_annotation(Annotation::primary(modifier.span())) - .with_annotation(Annotation::primary(last_static).with_message("`static` modifier")) + .with_annotation( + Annotation::primary(modifier.span()) + .with_message("`readonly` modifier cannot be used on static properties."), + ) + .with_annotation( + Annotation::primary(last_static).with_message("Property is marked as static here."), + ) .with_annotation( Annotation::secondary(first_variable.span()) - .with_message(format!("property `{}`", first_variable_name)), + .with_message(format!("Property `{}` declared here.", first_variable_name)), ) .with_annotation( Annotation::secondary(class_like_span) - .with_message(format!("{} `{}` defined here", class_like_kind, class_like_fqcn)), + .with_message(format!("{} `{}` defined here.", class_like_kind, class_like_fqcn)), ), ); } @@ -277,20 +323,22 @@ impl SemanticsWalker { if let Some(last_readonly) = last_readonly { context.report( Issue::error(format!( - "property `{}::{}` has multiple `readonly` modifiers", + "Property `{}::{}` has multiple `readonly` modifiers.", class_like_name, first_variable_name )) - .with_annotation(Annotation::primary(modifier.span())) .with_annotation( - Annotation::secondary(last_readonly).with_message("previous `readonly` modifier"), + Annotation::primary(modifier.span()).with_message("Duplicate `readonly` modifier."), + ) + .with_annotation( + Annotation::secondary(last_readonly).with_message("Previous `readonly` modifier."), ) .with_annotation( Annotation::secondary(first_variable.span()) - .with_message(format!("property `{}`", first_variable_name)), + .with_message(format!("Property `{}` declared here.", first_variable_name)), ) .with_annotation( Annotation::secondary(class_like_span) - .with_message(format!("{} `{}` defined here", class_like_kind, class_like_fqcn)), + .with_message(format!("{} `{}` defined here.", class_like_kind, class_like_fqcn)), ), ); } @@ -300,18 +348,20 @@ impl SemanticsWalker { Modifier::Final(_) => { if let Some(last_final) = last_final { context.report( - Issue::error("property has multiple `final` modifiers") - .with_annotation(Annotation::primary(modifier.span())) + Issue::error("Property has multiple `final` modifiers.") .with_annotation( - Annotation::primary(last_final).with_message("previous `final` modifier"), + Annotation::primary(modifier.span()).with_message("Duplicate `final` modifier."), + ) + .with_annotation( + Annotation::primary(last_final).with_message("Previous `final` modifier."), ) .with_annotation( Annotation::secondary(first_variable.span()) - .with_message(format!("property `{}`", first_variable_name)), + .with_message(format!("Property `{}` declared here.", first_variable_name)), ) .with_annotation( Annotation::secondary(class_like_span).with_message(format!( - "{} `{}` defined here", + "{} `{}` defined here.", class_like_kind, class_like_fqcn )), ), @@ -324,20 +374,22 @@ impl SemanticsWalker { if let Some(last_visibility) = last_read_visibility { context.report( Issue::error(format!( - "property `{}::{}` has multiple visibility modifiers", + "Property `{}::{}` has multiple visibility modifiers.", class_like_name, first_variable_name )) - .with_annotation(Annotation::primary(modifier.span())) .with_annotation( - Annotation::primary(last_visibility).with_message("previous visibility modifier"), + Annotation::primary(modifier.span()).with_message("Duplicate visibility modifier."), + ) + .with_annotation( + Annotation::primary(last_visibility).with_message("Previous visibility modifier."), ) .with_annotation( Annotation::secondary(first_variable.span()) - .with_message(format!("property `{}`", first_variable_name)), + .with_message(format!("Property `{}` declared here.", first_variable_name)), ) .with_annotation( Annotation::secondary(class_like_span) - .with_message(format!("{} `{}` defined here", class_like_kind, class_like_fqcn)), + .with_message(format!("{} `{}` defined here.", class_like_kind, class_like_fqcn)), ), ); } @@ -348,20 +400,24 @@ impl SemanticsWalker { if let Some(last_visibility) = last_write_visibility { context.report( Issue::error(format!( - "property `{}::{}` has multiple write visibility modifiers", + "Property `{}::{}` has multiple write visibility modifiers.", class_like_name, first_variable_name )) - .with_annotation(Annotation::primary(modifier.span())) .with_annotation( - Annotation::primary(last_visibility).with_message("previous write visibility modifier"), + Annotation::primary(modifier.span()) + .with_message("Duplicate write visibility modifier."), + ) + .with_annotation( + Annotation::primary(last_visibility) + .with_message("Previous write visibility modifier."), ) .with_annotation( Annotation::secondary(first_variable.span()) - .with_message(format!("property `{}`", first_variable_name)), + .with_message(format!("Property `{}` declared here.", first_variable_name)), ) .with_annotation( Annotation::secondary(class_like_span) - .with_message(format!("{} `{}` defined here", class_like_kind, class_like_fqcn)), + .with_message(format!("{} `{}` defined here.", class_like_kind, class_like_fqcn)), ), ); } @@ -369,18 +425,22 @@ impl SemanticsWalker { if let Some(last_static) = last_static { context.report( Issue::error(format!( - "static property `{}::{}` cannot have a write visibility modifier", + "Static property `{}::{}` cannot have a write visibility modifier.", class_like_name, first_variable_name )) - .with_annotation(Annotation::primary(modifier.span())) - .with_annotation(Annotation::primary(last_static).with_message("`static` modifier")) + .with_annotation( + Annotation::primary(modifier.span()).with_message("Write visibility modifier."), + ) + .with_annotation( + Annotation::primary(last_static).with_message("Property is marked as static here."), + ) .with_annotation( Annotation::secondary(first_variable.span()) - .with_message(format!("property `{}`", first_variable_name)), + .with_message(format!("Property `{}` declared here.", first_variable_name)), ) .with_annotation( Annotation::secondary(class_like_span) - .with_message(format!("{} `{}` defined here", class_like_kind, class_like_fqcn)), + .with_message(format!("{} `{}` defined here.", class_like_kind, class_like_fqcn)), ), ); } @@ -397,20 +457,22 @@ impl SemanticsWalker { context.report( Issue::error(format!( - "var property `{}::{}` cannot have modifiers", + "Var property `{}::{}` cannot have modifiers.", class_like_name, first_variable_name )) - .with_annotation(Annotation::primary(first.span().join(last.span()))) - .with_annotation(Annotation::primary(var.span()).with_message("`var` is here")) + .with_annotation( + Annotation::primary(first.span().join(last.span())).with_message("Modifiers used here."), + ) + .with_annotation(Annotation::primary(var.span()).with_message("Property is marked as `var` here.")) .with_annotation( Annotation::secondary(first_variable.span()) - .with_message(format!("property `{}`", first_variable_name)), + .with_message(format!("Property `{}` declared here.", first_variable_name)), ) .with_annotation( Annotation::secondary(class_like_span) - .with_message(format!("{} `{}` defined here", class_like_kind, class_like_fqcn)), + .with_message(format!("{} `{}` defined here.", class_like_kind, class_like_fqcn)), ) - .with_help("remove either the `var` keyword, or the modifiers".to_string()), + .with_help("Remove either the `var` keyword, or the modifiers.".to_string()), ); } } @@ -421,20 +483,20 @@ impl SemanticsWalker { // cant be used on properties context.report( Issue::error(format!( - "property `{}::{}` cannot have type `{}`", + "Property `{}::{}` cannot have type `{}`.", class_like_name, first_variable_name, hint_name )) .with_annotation( Annotation::primary(hint.span()) - .with_message(format!("type `{}` is not allowed on properties", hint_name)), + .with_message(format!("Type `{}` is not allowed on properties.", hint_name)), ) .with_annotation( Annotation::secondary(first_variable.span()) - .with_message(format!("property `{}`", first_variable_name)), + .with_message(format!("Property `{}` declared here.", first_variable_name)), ) .with_annotation( Annotation::secondary(class_like_span) - .with_message(format!("{} `{}` defined here", class_like_kind, class_like_fqcn)), + .with_message(format!("{} `{}` defined here.", class_like_kind, class_like_fqcn)), ), ); } @@ -442,17 +504,17 @@ impl SemanticsWalker { // readonly properties must have a type hint context.report( Issue::error(format!( - "readonly property `{}::{}` must have a type hint", + "Readonly property `{}::{}` must have a type hint.", class_like_name, first_variable_name )) - .with_annotation(Annotation::primary(readonly)) + .with_annotation(Annotation::primary(readonly).with_message("Property is marked as readonly here.")) .with_annotation( Annotation::secondary(first_variable.span()) - .with_message(format!("property `{}`", first_variable_name)), + .with_message(format!("Property `{}` declared here.", first_variable_name)), ) .with_annotation( Annotation::secondary(class_like_span) - .with_message(format!("{} `{}` defined here", class_like_kind, class_like_fqcn)), + .with_message(format!("{} `{}` defined here.", class_like_kind, class_like_fqcn)), ), ); } @@ -467,20 +529,20 @@ impl SemanticsWalker { if !property_concrete_item.value.is_constant(false) { context.report( Issue::error(format!( - "property `{}::{}` value contains a non-constant expression", + "Property `{}::{}` value contains a non-constant expression.", class_like_name, item_name )) .with_annotation( Annotation::primary(property_concrete_item.value.span()) - .with_message("non-constant expression"), + .with_message("This is a non-constant expression."), ) .with_annotation( Annotation::secondary(property_concrete_item.variable.span()) - .with_message(format!("property `{}` defined here", item_name)), + .with_message(format!("Property `{}` declared here.", item_name)), ) .with_annotation( Annotation::secondary(class_like_span).with_message(format!( - "{} `{}` defined here", + "{} `{}` defined here.", class_like_kind, class_like_fqcn )), ), @@ -490,24 +552,24 @@ impl SemanticsWalker { if let Some(readonly) = last_readonly { context.report( Issue::error(format!( - "readonly property `{}::{}` cannot have a default value", + "Readonly property `{}::{}` cannot have a default value.", class_like_name, item_name )) .with_annotation( Annotation::primary(property_concrete_item.value.span()) - .with_message("default value here"), + .with_message("This is a default value."), ) .with_annotation(Annotation::primary(readonly).with_message(format!( - "property `{}::{}` is marked as readonly", + "Property `{}::{}` is marked as readonly here.", class_like_name, item_name ))) .with_annotation( Annotation::secondary(property_concrete_item.variable.span()) - .with_message(format!("property `{}` defined here", item_name)), + .with_message(format!("Property `{}` is declared here.", item_name)), ) .with_annotation( Annotation::secondary(class_like_span).with_message(format!( - "{} `{}` defined here", + "{} `{}` defined here.", class_like_kind, class_like_fqcn )), ), @@ -523,40 +585,46 @@ impl SemanticsWalker { if let Some(readonly) = last_readonly { context.report( Issue::error(format!( - "hooked property `{}::{}` cannot be readonly", + "Hooked property `{}::{}` cannot be readonly.", class_like_name, item_name )) - .with_annotation(Annotation::primary(hooked_property.hooks.span())) .with_annotation(Annotation::primary(readonly).with_message(format!( - "property `{}::{}` is marked as readonly", + "Property `{}::{}` is marked as readonly here.", class_like_name, item_name ))) + .with_annotation( + Annotation::secondary(hooked_property.hooks.span()) + .with_message("Property hooks are defined here."), + ) .with_annotation( Annotation::secondary(hooked_property.item.variable().span()) - .with_message(format!("property `{}` defined here", item_name)), + .with_message(format!("Property `{}` is declared here.", item_name)), ) .with_annotation( Annotation::secondary(class_like_span) - .with_message(format!("{} `{}` defined here", class_like_kind, class_like_fqcn)), + .with_message(format!("{} `{}` defined here.", class_like_kind, class_like_fqcn)), ), ); } if let Some(r#static) = last_static { context.report( - Issue::error(format!("hooked property `{}::{}` cannot be static", class_like_name, item_name)) - .with_annotation(Annotation::primary(hooked_property.hooks.span())) + Issue::error(format!("Hooked property `{}::{}` cannot be static.", class_like_name, item_name)) .with_annotation(Annotation::primary(r#static).with_message(format!( - "property `{}::{}` is marked as static", + "Property `{}::{}` is marked as static here.", class_like_name, item_name ))) + .with_annotation( + Annotation::secondary(hooked_property.hooks.span()) + .with_message("Property hooks are defined here."), + ) .with_annotation( Annotation::secondary(hooked_property.item.variable().span()) - .with_message(format!("property `{}` defined here", item_name)), + .with_message(format!("Property `{}` is declared here.", item_name)), ) .with_annotation( Annotation::secondary(class_like_span) - .with_message(format!("{} `{}` defined here", class_like_kind, class_like_fqcn)), + .with_message(format!("{} `{}` defined here.", class_like_kind, class_like_fqcn)), ), ); } @@ -572,19 +640,20 @@ impl SemanticsWalker { context.report( Issue::error(format!( - "hook `{}` for property `{}::{}` cannot have modifiers", + "Hook `{}` for property `{}::{}` cannot have modifiers.", name, class_like_name, item_name )) .with_annotation( - Annotation::primary(first.span().join(last.span())).with_message("hook modifiers here"), + Annotation::primary(first.span().join(last.span())) + .with_message("Hook modifiers here."), ) .with_annotation( Annotation::secondary(hooked_property.item.variable().span()) - .with_message(format!("property `{}` defined here", item_name)), + .with_message(format!("Property `{}` is declared here.", item_name)), ) .with_annotation( Annotation::secondary(class_like_span) - .with_message(format!("{} `{}` defined here", class_like_kind, class_like_fqcn)), + .with_message(format!("{} `{}` defined here.", class_like_kind, class_like_fqcn)), ), ); } @@ -592,18 +661,21 @@ impl SemanticsWalker { if !class_like_is_interface { if let PropertyHookBody::Abstract(property_hook_abstract_body) = &hook.body { context.report( - Issue::error(format!("non-abstract property hook `{}` must have a body", name)) - .with_annotation(Annotation::primary(property_hook_abstract_body.span())) + Issue::error(format!("Non-abstract property hook `{}` must have a body.", name)) + .with_annotation( + Annotation::primary(property_hook_abstract_body.span()) + .with_message("Abstract hook body here."), + ) .with_annotation( Annotation::secondary(hook.name.span()) - .with_message(format!("hook `{}` defined here", name)), + .with_message(format!("Hook `{}` is declared here.", name)), ) .with_annotation( Annotation::secondary(hooked_property.item.variable().span()) - .with_message(format!("property `{}` defined here", item_name)), + .with_message(format!("Property `{}` is declared here.", item_name)), ) .with_annotation(Annotation::secondary(class_like_span).with_message(format!( - "{} `{}` defined here", + "{} `{}` defined here.", class_like_kind, class_like_fqcn ))), ); @@ -616,22 +688,22 @@ impl SemanticsWalker { if parameters.parameters.len() != 1 { context.report( Issue::error(format!( - "hook `{}` of property `{}::{}` must accept exactly one parameter, found {}", + "Hook `{}` of property `{}::{}` must accept exactly one parameter, found {}.", name, class_like_name, item_name, parameters.parameters.len() )) - .with_annotation(Annotation::primary(parameters.span())) + .with_annotation(Annotation::primary(parameters.span()).with_message("Parameters are defined here.")) .with_annotation( Annotation::secondary(hook.name.span()).with_message( - format!("hook `{}` defined here", name), + format!("Hook `{}` is declared here.", name), ), ) .with_annotation( Annotation::secondary(hooked_property.item.variable().span()) - .with_message(format!("property `{}` defined here", item_name)), + .with_message(format!("Property `{}` is declared here.", item_name)), ) .with_annotation( Annotation::secondary(class_like_span) - .with_message(format!("{} `{}` defined here", class_like_kind, class_like_fqcn)), + .with_message(format!("{} `{}` defined here.", class_like_kind, class_like_fqcn)), ), ); } else { @@ -641,21 +713,28 @@ impl SemanticsWalker { if first_parameter.hint.is_none() { context.report( Issue::error(format!( - "parameter `{}` of hook `{}::{}::{}` must contain a type hint", + "Parameter `{}` of hook `{}::{}::{}` must contain a type hint.", first_parameter_name, class_like_name, item_name, name )) - .with_annotation(Annotation::primary(first_parameter.variable.span())) + .with_annotation( + Annotation::primary(first_parameter.variable.span()).with_message( + format!("Parameter `{}` declared here.", first_parameter_name), + ), + ) .with_annotation( Annotation::secondary(hook.name.span()) - .with_message(format!("hook `{}` defined here", name)), + .with_message(format!("Hook `{}` is declared here.", name)), ) .with_annotation( Annotation::secondary(hooked_property.item.variable().span()) - .with_message(format!("property `{}` defined here", item_name)), + .with_message(format!( + "Property `{}` is declared here.", + item_name + )), ) .with_annotation( Annotation::secondary(class_like_span).with_message(format!( - "{} `{}` defined here", + "{} `{}` defined here.", class_like_kind, class_like_fqcn )), ), @@ -665,26 +744,34 @@ impl SemanticsWalker { if let Some(ellipsis) = first_parameter.ellipsis { context.report( Issue::error(format!( - "parameter `{}` of hook `{}::{}::{}` must not be variadic", + "Parameter `{}` of hook `{}::{}::{}` must not be variadic.", first_parameter_name, class_like_name, item_name, name )) - .with_annotation(Annotation::primary(ellipsis.span())) + .with_annotation(Annotation::primary(ellipsis.span()).with_message( + format!( + "Parameter `{}` is marked as variadic here.", + first_parameter_name + ), + )) .with_annotation( Annotation::secondary(first_parameter.variable.span()).with_message( - format!("parameter `{}` defined here", first_parameter_name), + format!("Parameter `{}` declared here.", first_parameter_name), ), ) .with_annotation( Annotation::secondary(hook.name.span()) - .with_message(format!("hook `{}` defined here", name)), + .with_message(format!("Hook `{}` is declared here.", name)), ) .with_annotation( Annotation::secondary(hooked_property.item.variable().span()) - .with_message(format!("property `{}` defined here", item_name)), + .with_message(format!( + "Property `{}` is declared here.", + item_name + )), ) .with_annotation( Annotation::secondary(class_like_span).with_message(format!( - "{} `{}` defined here", + "{} `{}` defined here.", class_like_kind, class_like_fqcn )), ), @@ -694,26 +781,34 @@ impl SemanticsWalker { if let Some(ampersand) = first_parameter.ampersand { context.report( Issue::error(format!( - "parameter `{}` of hook `{}::{}::{}` must not be pass-by-reference", + "Parameter `{}` of hook `{}::{}::{}` must not be pass-by-reference.", first_parameter_name, class_like_name, item_name, name )) - .with_annotation(Annotation::primary(ampersand.span())) + .with_annotation(Annotation::primary(ampersand.span()).with_message( + format!( + "Parameter `{}` is marked as pass-by-reference here.", + first_parameter_name + ), + )) .with_annotation( Annotation::secondary(first_parameter.variable.span()).with_message( - format!("parameter `{}` defined here", first_parameter_name), + format!("Parameter `{}` declared here.", first_parameter_name), ), ) .with_annotation( Annotation::secondary(hook.name.span()) - .with_message(format!("hook `{}` defined here", name)), + .with_message(format!("Hook `{}` is declared here.", name)), ) .with_annotation( Annotation::secondary(hooked_property.item.variable().span()) - .with_message(format!("property `{}` defined here", item_name)), + .with_message(format!( + "Property `{}` is declared here.", + item_name + )), ) .with_annotation( Annotation::secondary(class_like_span).with_message(format!( - "{} `{}` defined here", + "{} `{}` defined here.", class_like_kind, class_like_fqcn )), ), @@ -723,26 +818,29 @@ impl SemanticsWalker { if let Some(default_value) = &first_parameter.default_value { context.report( Issue::error(format!( - "parameter `{}` of hook `{}::{}::{}` must not have a default value", + "Parameter `{}` of hook `{}::{}::{}` must not have a default value.", first_parameter_name, class_like_name, item_name, name )) .with_annotation(Annotation::primary(default_value.span())) .with_annotation( Annotation::secondary(first_parameter.variable.span()).with_message( - format!("parameter `{}` defined here", first_parameter_name), + format!("Parameter `{}` declared here.", first_parameter_name), ), ) .with_annotation( Annotation::secondary(hook.name.span()) - .with_message(format!("hook `{}` defined here", name)), + .with_message(format!("Hook `{}` is declared here.", name)), ) .with_annotation( Annotation::secondary(hooked_property.item.variable().span()) - .with_message(format!("property `{}` defined here", item_name)), + .with_message(format!( + "Property `{}` is declared here.", + item_name + )), ) .with_annotation( Annotation::secondary(class_like_span).with_message(format!( - "{} `{}` defined here", + "{} `{}` defined here.", class_like_kind, class_like_fqcn )), ), @@ -755,21 +853,24 @@ impl SemanticsWalker { if let Some(parameters) = &hook.parameters { context.report( Issue::error(format!( - "hook `{}` of property `{}::{}` must not have a parameters list", + "Hook `{}` of property `{}::{}` must not have a parameters list.", name, class_like_name, item_name )) - .with_annotation(Annotation::primary(parameters.span())) + .with_annotation( + Annotation::primary(parameters.span()) + .with_message("Parameters are defined here."), + ) .with_annotation( Annotation::secondary(hook.name.span()) - .with_message(format!("hook `{}` defined here", name)), + .with_message(format!("Hook `{}` is declared here.", name)), ) .with_annotation( Annotation::secondary(hooked_property.item.variable().span()) - .with_message(format!("property `{}` defined here", item_name)), + .with_message(format!("Property `{}` is declared here.", item_name)), ) .with_annotation( Annotation::secondary(class_like_span).with_message(format!( - "{} `{}` defined here", + "{} `{}` defined here.", class_like_kind, class_like_fqcn )), ), @@ -779,20 +880,20 @@ impl SemanticsWalker { _ => { context.report( Issue::error(format!( - "hooked property `{}::{}` contains an unknwon hook `{}`, expected `set` or `get`", + "Hooked property `{}::{}` contains an unknwon hook `{}`, expected `set` or `get`.", class_like_name, item_name, name )) .with_annotation( Annotation::primary(hook.name.span()) - .with_message(format!("hook `{}` defined here", name)), + .with_message(format!("Hook `{}` declared here.", name)), ) .with_annotation( Annotation::secondary(hooked_property.item.variable().span()) - .with_message(format!("property `{}` defined here", item_name)), + .with_message(format!("Property `{}` is declared here.", item_name)), ) .with_annotation( Annotation::secondary(class_like_span).with_message(format!( - "{} `{}` defined here", + "{} `{}` defined here.", class_like_kind, class_like_fqcn )), ), @@ -804,21 +905,24 @@ impl SemanticsWalker { { context.report( Issue::error(format!( - "hook `{}` has already been defined for property `{}::{}`", + "Hook `{}` has already been defined for property `{}::{}`.", name, class_like_name, item_name )) - .with_annotation(Annotation::primary(hook.name.span())) + .with_annotation( + Annotation::primary(hook.name.span()) + .with_message(format!("Duplicate hook `{}`.", name)), + ) .with_annotation( Annotation::secondary(*previous_span) - .with_message(format!("previous definition of hook `{}`", previous_span)), + .with_message(format!("Previous declaration of hook `{}`", previous_span)), ) .with_annotation( Annotation::secondary(hooked_property.item.variable().span()) - .with_message(format!("property `{}` defined here", item_name)), + .with_message(format!("Property `{}` is declared here.", item_name)), ) .with_annotation( Annotation::secondary(class_like_span) - .with_message(format!("{} `{}` defined here", class_like_kind, class_like_fqcn)), + .with_message(format!("{} `{}` defined here.", class_like_kind, class_like_fqcn)), ), ); } else { @@ -829,6 +933,7 @@ impl SemanticsWalker { }; } + #[inline] fn process_method( &self, method: &Method, @@ -862,13 +967,15 @@ impl SemanticsWalker { ) .with_annotation( Annotation::secondary(method.span()).with_message(format!( - "method `{}::{}` defined here", + "method `{}::{}` defined here.", class_like_name, method_name, )), ) .with_annotation( - Annotation::secondary(class_like_span) - .with_message(format!("{} `{}` is defined here", class_like_kind, class_like_fqcn)), + Annotation::secondary(class_like_span).with_message(format!( + "{} `{}` is defined here.", + class_like_kind, class_like_fqcn + )), ), ); } @@ -886,13 +993,15 @@ impl SemanticsWalker { .with_annotation(Annotation::primary(abstract_modifier).with_message("`abstract` modifier")) .with_annotation( Annotation::secondary(method.span()).with_message(format!( - "method `{}::{}` defined here", + "method `{}::{}` defined here.", class_like_name, method_name, )), ) .with_annotation( - Annotation::secondary(class_like_span) - .with_message(format!("{} `{}` is defined here", class_like_kind, class_like_fqcn)), + Annotation::secondary(class_like_span).with_message(format!( + "{} `{}` is defined here.", + class_like_kind, class_like_fqcn + )), ), ); } @@ -909,13 +1018,15 @@ impl SemanticsWalker { .with_annotation(Annotation::primary(last_final).with_message("previous `final` modifier")) .with_annotation( Annotation::secondary(method.span()).with_message(format!( - "method `{}::{}` defined here", + "method `{}::{}` defined here.", class_like_name, method_name, )), ) .with_annotation( - Annotation::secondary(class_like_span) - .with_message(format!("{} `{}` is defined here", class_like_kind, class_like_fqcn)), + Annotation::secondary(class_like_span).with_message(format!( + "{} `{}` is defined here.", + class_like_kind, class_like_fqcn + )), ), ); } @@ -933,13 +1044,15 @@ impl SemanticsWalker { .with_annotation(Annotation::primary(final_modifier).with_message("`final` modifier")) .with_annotation( Annotation::secondary(method.span()).with_message(format!( - "method `{}::{}` defined here", + "method `{}::{}` defined here.", class_like_name, method_name, )), ) .with_annotation( - Annotation::secondary(class_like_span) - .with_message(format!("{} `{}` is defined here", class_like_kind, class_like_fqcn)), + Annotation::secondary(class_like_span).with_message(format!( + "{} `{}` is defined here.", + class_like_kind, class_like_fqcn + )), ), ); } @@ -958,13 +1071,15 @@ impl SemanticsWalker { ) .with_annotation( Annotation::secondary(method.span()).with_message(format!( - "method `{}::{}` defined here", + "method `{}::{}` defined here.", class_like_name, method_name, )), ) .with_annotation( - Annotation::secondary(class_like_span) - .with_message(format!("{} `{}` is defined here", class_like_kind, class_like_fqcn)), + Annotation::secondary(class_like_span).with_message(format!( + "{} `{}` is defined here.", + class_like_kind, class_like_fqcn + )), ), ); } @@ -977,13 +1092,15 @@ impl SemanticsWalker { .with_annotation(Annotation::primary(modifier.span()).with_message("`readonly` modifier")) .with_annotation( Annotation::secondary(method.span()).with_message(format!( - "method `{}::{}` defined here", + "method `{}::{}` defined here.", class_like_name, method_name, )), ) .with_annotation( - Annotation::secondary(class_like_span) - .with_message(format!("{} `{}` is defined here", class_like_kind, class_like_fqcn)), + Annotation::secondary(class_like_span).with_message(format!( + "{} `{}` is defined here.", + class_like_kind, class_like_fqcn + )), ), ); } @@ -1002,13 +1119,15 @@ impl SemanticsWalker { ) .with_annotation( Annotation::secondary(method.span()).with_message(format!( - "method `{}::{}` defined here", + "method `{}::{}` defined here.", class_like_name, method_name, )), ) .with_annotation( - Annotation::secondary(class_like_span) - .with_message(format!("{} `{}` is defined here", class_like_kind, class_like_fqcn)), + Annotation::secondary(class_like_span).with_message(format!( + "{} `{}` is defined here.", + class_like_kind, class_like_fqcn + )), ), ); } else { @@ -1027,13 +1146,15 @@ impl SemanticsWalker { ) .with_annotation( Annotation::secondary(method.span()).with_message(format!( - "method `{}::{}` defined here", + "method `{}::{}` defined here.", class_like_name, method_name, )), ) .with_annotation( - Annotation::secondary(class_like_span) - .with_message(format!("{} `{}` is defined here", class_like_kind, class_like_fqcn)), + Annotation::secondary(class_like_span).with_message(format!( + "{} `{}` is defined here.", + class_like_kind, class_like_fqcn + )), ), ); } @@ -1075,11 +1196,11 @@ impl SemanticsWalker { Issue::error(message) .with_annotation(Annotation::primary(method.parameters.span())) .with_annotation(Annotation::secondary(method.span()).with_message(format!( - "method `{}::{}` defined here", + "method `{}::{}` defined here.", class_like_name, method_name, ))) .with_annotation(Annotation::secondary(class_like_span).with_message(format!( - "{} `{}` is defined here", + "{} `{}` is defined here.", class_like_kind, class_like_fqcn ))), ); @@ -1095,13 +1216,15 @@ impl SemanticsWalker { ) .with_annotation( Annotation::secondary(method.span()).with_message(format!( - "method `{}::{}` defined here", + "method `{}::{}` defined here.", class_like_name, method_name, )), ) .with_annotation( - Annotation::secondary(class_like_span) - .with_message(format!("{} `{}` is defined here", class_like_kind, class_like_fqcn)), + Annotation::secondary(class_like_span).with_message(format!( + "{} `{}` is defined here.", + class_like_kind, class_like_fqcn + )), ), ); } @@ -1116,13 +1239,15 @@ impl SemanticsWalker { .with_annotation(Annotation::primary(*span).with_message("`static` modifier")) .with_annotation( Annotation::secondary(method.span()).with_message(format!( - "method `{}::{}` defined here", + "method `{}::{}` defined here.", class_like_name, method_name, )), ) .with_annotation( - Annotation::secondary(class_like_span) - .with_message(format!("{} `{}` is defined here", class_like_kind, class_like_fqcn)), + Annotation::secondary(class_like_span).with_message(format!( + "{} `{}` is defined here.", + class_like_kind, class_like_fqcn + )), ), ); } @@ -1135,7 +1260,7 @@ impl SemanticsWalker { .with_message(format!("{} `{}`", class_like_kind, class_like_fqcn)), ) .with_annotation(Annotation::secondary(method.span()).with_message(format!( - "method `{}::{}` defined here", + "method `{}::{}` defined here.", class_like_name, method_name, ))), ); @@ -1153,13 +1278,15 @@ impl SemanticsWalker { .with_annotation(Annotation::primary(hint.span())) .with_annotation( Annotation::secondary(method.span()).with_message(format!( - "method `{}::{}` defined here", + "method `{}::{}` defined here.", class_like_name, method_name, )), ) .with_annotation( - Annotation::secondary(class_like_span) - .with_message(format!("{} `{}` is defined here", class_like_kind, class_like_fqcn)), + Annotation::secondary(class_like_span).with_message(format!( + "{} `{}` is defined here.", + class_like_kind, class_like_fqcn + )), ), ); } @@ -1178,9 +1305,9 @@ impl SemanticsWalker { .with_annotation(Annotation::primary(method_abstract_body.span())) .with_annotations([ Annotation::secondary(class_like_span) - .with_message(format!("{} `{}` is defined here", class_like_kind, class_like_fqcn)), + .with_message(format!("{} `{}` is defined here.", class_like_kind, class_like_fqcn)), Annotation::secondary(method.span()) - .with_message(format!("method `{}::{}` defined here", class_like_name, method_name)), + .with_message(format!("method `{}::{}` defined here.", class_like_name, method_name)), ]), ); } @@ -1196,9 +1323,9 @@ impl SemanticsWalker { .with_annotations([ Annotation::primary(abstract_modifier.span()), Annotation::secondary(class_like_span) - .with_message(format!("{} `{}` is defined here", class_like_kind, class_like_fqcn)), + .with_message(format!("{} `{}` is defined here.", class_like_kind, class_like_fqcn)), Annotation::secondary(method.span()) - .with_message(format!("method `{}::{}` defined here", class_like_name, method_name)), + .with_message(format!("method `{}::{}` defined here.", class_like_name, method_name)), ]), ); } else if class_like_is_interface { @@ -1210,9 +1337,9 @@ impl SemanticsWalker { .with_annotation(Annotation::primary(body.span())) .with_annotations([ Annotation::secondary(class_like_span) - .with_message(format!("{} `{}` is defined here", class_like_kind, class_like_fqcn)), + .with_message(format!("{} `{}` is defined here.", class_like_kind, class_like_fqcn)), Annotation::secondary(method.span()) - .with_message(format!("method `{}::{}` defined here", class_like_name, method_name)), + .with_message(format!("method `{}::{}` defined here.", class_like_name, method_name)), ]), ); } @@ -1237,11 +1364,11 @@ impl SemanticsWalker { .with_annotation(Annotation::primary(val.span())) .with_annotations([ Annotation::secondary(class_like_span).with_message(format!( - "{} `{}` is defined here", + "{} `{}` is defined here.", class_like_kind, class_like_fqcn )), Annotation::secondary(method.span()).with_message(format!( - "method `{}::{}` defined here", + "method `{}::{}` defined here.", class_like_name, method_name, )), ]) @@ -1260,11 +1387,11 @@ impl SemanticsWalker { .with_annotation(Annotation::primary(r#return.span())) .with_annotations([ Annotation::secondary(class_like_span).with_message(format!( - "{} `{}` is defined here", + "{} `{}` is defined here.", class_like_kind, class_like_fqcn )), Annotation::secondary(method.span()).with_message(format!( - "method `{}::{}` defined here", + "method `{}::{}` defined here.", class_like_name, method_name, )), ]) @@ -1283,11 +1410,11 @@ impl SemanticsWalker { .with_annotation(Annotation::primary(r#return.span())) .with_annotations([ Annotation::secondary(class_like_span).with_message(format!( - "{} `{}` is defined here", + "{} `{}` is defined here.", class_like_kind, class_like_fqcn )), Annotation::secondary(method.span()).with_message(format!( - "method `{}::{}` defined here", + "method `{}::{}` defined here.", class_like_name, method_name, )), ]) @@ -1303,7 +1430,7 @@ impl SemanticsWalker { }; } - #[inline(always)] + #[inline] fn process_members( &self, members: &Sequence, @@ -1342,11 +1469,11 @@ impl SemanticsWalker { .with_annotation(Annotation::primary(item.variable().span())) .with_annotations([ Annotation::secondary(*span).with_message(format!( - "property `{}::{}` previously defined here", + "property `{}::{}` previously defined here.", class_like_name, item_name )), Annotation::secondary(class_like_span.span()).with_message(format!( - "{} `{}` defined here", + "{} `{}` defined here.", class_like_kind, class_like_fqcn )), ]) @@ -1379,11 +1506,11 @@ impl SemanticsWalker { .with_annotation(Annotation::primary(item_variable.span())) .with_annotations([ Annotation::secondary(*span).with_message(format!( - "property `{}::{}` previously defined here", + "property `{}::{}` previously defined here.", class_like_name, item_name )), Annotation::secondary(class_like_span.span()).with_message(format!( - "{} `{}` defined here", + "{} `{}` defined here.", class_like_kind, class_like_fqcn )), ]) @@ -1411,7 +1538,7 @@ impl SemanticsWalker { .with_annotations([ Annotation::secondary(*previous).with_message("previous definition"), Annotation::secondary(class_like_span.span()) - .with_message(format!("{} `{}` defined here", class_like_kind, class_like_fqcn)), + .with_message(format!("{} `{}` defined here.", class_like_kind, class_like_fqcn)), ]), ); } else { @@ -1444,11 +1571,11 @@ impl SemanticsWalker { .with_annotation(Annotation::primary(parameter.variable.span())) .with_annotations([ Annotation::secondary(*span).with_message(format!( - "property `{}::{}` previously defined here", + "property `{}::{}` previously defined here.", class_like_name, item_name )), Annotation::secondary(class_like_span.span()).with_message(format!( - "{} `{}` defined here", + "{} `{}` defined here.", class_like_kind, class_like_fqcn )), ]) @@ -1475,11 +1602,11 @@ impl SemanticsWalker { .with_annotation(Annotation::primary(item.name.span())) .with_annotations([ Annotation::secondary(*span).with_message(format!( - "constant `{}::{}` previously defined here", + "Constant `{}::{}` previously defined here.", class_like_name, name )), Annotation::secondary(class_like_span.span()).with_message(format!( - "{} `{}` defined here", + "{} `{}` defined here.", class_like_kind, class_like_fqcn )), ]), @@ -1492,10 +1619,12 @@ impl SemanticsWalker { )) .with_annotation(Annotation::primary(item.name.span())) .with_annotations([ - Annotation::secondary(*span) - .with_message(format!("case `{}::{}` defined here", class_like_name, name)), + Annotation::secondary(*span).with_message(format!( + "case `{}::{}` defined here.", + class_like_name, name + )), Annotation::secondary(class_like_span.span()).with_message(format!( - "{} `{}` defined here", + "{} `{}` defined here.", class_like_kind, class_like_fqcn )), ]), @@ -1518,10 +1647,12 @@ impl SemanticsWalker { )) .with_annotation(Annotation::primary(enum_case.item.name().span())) .with_annotations([ - Annotation::secondary(*span) - .with_message(format!("constant `{}::{}` defined here", class_like_name, name)), + Annotation::secondary(*span).with_message(format!( + "Constant `{}::{}` defined here.", + class_like_name, name + )), Annotation::secondary(class_like_span.span()).with_message(format!( - "{} `{}` defined here", + "{} `{}` defined here.", class_like_kind, class_like_fqcn )), ]), @@ -1535,11 +1666,11 @@ impl SemanticsWalker { .with_annotation(Annotation::primary(enum_case.item.name().span())) .with_annotations([ Annotation::secondary(*span).with_message(format!( - "case `{}::{}` previously defined here", + "case `{}::{}` previously defined here.", class_like_name, name )), Annotation::secondary(class_like_span.span()).with_message(format!( - "{} `{}` defined here", + "{} `{}` defined here.", class_like_kind, class_like_fqcn )), ]), @@ -1556,6 +1687,7 @@ impl SemanticsWalker { } } + #[inline] fn process_class_like_constant( &self, class_like_constant: &ClassLikeConstant, @@ -1581,11 +1713,11 @@ impl SemanticsWalker { .with_annotation(Annotation::primary(modifier.span())) .with_annotations([ Annotation::secondary(first_item.span()).with_message(format!( - "{} constant `{}::{}` is declared here", + "{} constant `{}::{}` is declared here.", class_like_kind, class_like_name, first_item_name )), Annotation::secondary(class_like_span) - .with_message(format!("{} `{}` is declared here", class_like_kind, class_like_fqcn)), + .with_message(format!("{} `{}` is declared here.", class_like_kind, class_like_fqcn)), ]), ); } @@ -1597,11 +1729,11 @@ impl SemanticsWalker { .with_annotations([ Annotation::secondary(last_final).with_message("previous `final` modifier"), Annotation::secondary(first_item.span()).with_message(format!( - "{} constant `{}::{}` is declared here", + "{} constant `{}::{}` is declared here.", class_like_kind, class_like_name, first_item_name )), Annotation::secondary(class_like_span).with_message(format!( - "{} `{}` is declared here", + "{} `{}` is declared here.", class_like_kind, class_like_fqcn )), ]), @@ -1618,11 +1750,11 @@ impl SemanticsWalker { .with_annotations([ Annotation::secondary(last_visibility).with_message("previous visibility modifier"), Annotation::secondary(first_item.span()).with_message(format!( - "{} constant `{}::{}` is declared here", + "{} constant `{}::{}` is declared here.", class_like_kind, class_like_name, first_item_name )), Annotation::secondary(class_like_span).with_message(format!( - "{} `{}` is declared here", + "{} `{}` is declared here.", class_like_kind, class_like_fqcn )), ]), @@ -1640,23 +1772,24 @@ impl SemanticsWalker { if !item.value.is_constant(false) { context.report( Issue::error(format!( - "constant `{}::{}` value contains a non-constant expression", + "Constant `{}::{}` value contains a non-constant expression.", class_like_name, item_name )) .with_annotation(Annotation::primary(item.value.span())) .with_annotations([ Annotation::secondary(item.name.span()).with_message(format!( - "{} constant `{}::{}` is declared here", + "{} constant `{}::{}` is declared here.", class_like_kind, class_like_name, item_name )), Annotation::secondary(class_like_span) - .with_message(format!("{} `{}` is declared here", class_like_kind, class_like_fqcn)), + .with_message(format!("{} `{}` is declared here.", class_like_kind, class_like_fqcn)), ]), ); } } } + #[inline] fn process_promoted_properties_outside_constructor( &self, parameter_list: &FunctionLikeParameterList, @@ -1665,8 +1798,11 @@ impl SemanticsWalker { for parameter in parameter_list.parameters.iter() { if parameter.is_promoted_property() { context.report( - Issue::error("promoted properties are not allowed outside of constructors") - .with_annotation(Annotation::primary(parameter.span())), + Issue::error("Promoted properties are not allowed outside of constructors.") + .with_annotation( + Annotation::primary(parameter.span()).with_message("Promoted property found here."), + ) + .with_help("Move this promoted property to the constructor, or remove the promotion."), ); } } @@ -1718,12 +1854,16 @@ impl Walker> for SemanticsWalker { if name.eq_ignore_ascii_case(STRICT_TYPES_DECLARE_DIRECTIVE) { context.report( - Issue::error("strict type declaration must be the first statement in the file") - .with_annotation(Annotation::primary(declare.span())) + Issue::error("Strict type declaration must be the first statement in the file.") + .with_annotation( + Annotation::primary(declare.span()) + .with_message("Strict type declaration found here."), + ) .with_annotations(before.iter().map(|span| { Annotation::secondary(*span) - .with_message("this statement should come after the strict type declaration") - })), + .with_message("This statement appears before the strict type declaration.") + })) + .with_help("Move all statements before the strict type declaration to after it."), ); } } @@ -1754,12 +1894,15 @@ impl Walker> for SemanticsWalker { if let Statement::Namespace(namespace) = statement { context.report( - Issue::error("namespace must be the first statement in the file") - .with_annotation(Annotation::primary(namespace.span())) + Issue::error("Namespace must be the first statement in the file.") + .with_annotation( + Annotation::primary(namespace.span()).with_message("Namespace statement found here."), + ) .with_annotations(before.iter().map(|span| { Annotation::secondary(*span) - .with_message("this statement should come after the namespace statement") - })), + .with_message("This statement appears before the namespace declaration.") + })) + .with_help("Move all statements before the namespace declaration to after it."), ); } } @@ -1780,25 +1923,37 @@ impl Walker> for SemanticsWalker { NamespaceBody::Implicit(body) => { if namespace.name.is_none() { context.report( - Issue::error("unbraced namespace must be named") - .with_annotation(Annotation::primary(namespace.span().join(body.terminator.span()))) - .with_annotation(Annotation::secondary(body.span())), + Issue::error("Unbraced namespace must be named.") + .with_annotation( + Annotation::primary(namespace.span().join(body.terminator.span())) + .with_message("Unnamed unbraced namespace."), + ) + .with_annotation( + Annotation::secondary(body.span()).with_message("Namespace body without a name."), + ) + .with_help("Add a name to the unbraced namespace."), ); } last_unbraced = Some((namespace_span, body.span())); - if let Some((last_namespace_span, last_body_span)) = last_braced { context.report( Issue::error( - "cannot mix unbraced namespace declarations with braced namespace declarations", + "Cannot mix unbraced namespace declarations with braced namespace declarations.", + ) + .with_annotation( + Annotation::primary(namespace_span) + .with_message("This is an unbraced namespace declaration."), ) - .with_annotation(Annotation::primary(namespace_span)) .with_annotations([ - Annotation::primary(last_namespace_span), - Annotation::secondary(last_body_span).with_message("braced namespace declaration"), - Annotation::secondary(body.span()).with_message("unbraced namespace declaration"), - ]), + Annotation::primary(last_namespace_span) + .with_message("Previous braced namespace declaration."), + Annotation::secondary(last_body_span).with_message("Braced namespace body."), + Annotation::secondary(body.span()).with_message("Unbraced namespace body."), + ]) + .with_help( + "Use consistent namespace declaration styles: either all braced or all unbraced.", + ), ); } } @@ -1808,14 +1963,21 @@ impl Walker> for SemanticsWalker { if let Some((last_namespace_span, last_body_span)) = last_unbraced { context.report( Issue::error( - "cannot mix braced namespace declarations with unbraced namespace declarations", + "Cannot mix braced namespace declarations with unbraced namespace declarations.", + ) + .with_annotation( + Annotation::primary(namespace_span) + .with_message("This is a braced namespace declaration."), ) - .with_annotation(Annotation::primary(namespace_span)) .with_annotations([ - Annotation::primary(last_namespace_span), - Annotation::secondary(last_body_span).with_message("unbraced namespace declaration"), - Annotation::secondary(body.span()).with_message("braced namespace declaration"), - ]), + Annotation::primary(last_namespace_span) + .with_message("Previous unbraced namespace declaration."), + Annotation::secondary(last_body_span).with_message("Unbraced namespace body."), + Annotation::secondary(body.span()).with_message("Braced namespace body."), + ]) + .with_help( + "Use consistent namespace declaration styles: either all braced or all unbraced.", + ), ); } } @@ -1825,9 +1987,12 @@ impl Walker> for SemanticsWalker { fn walk_in_short_opening_tag(&self, short_opening_tag: &ShortOpeningTag, context: &mut Context<'_>) { context.report( - Issue::error("short opening tag `> for SemanticsWalker { if !matches!(value, Some(0) | Some(1)) { context.report( - Issue::error(format!("`{}` declare directive must be set to either `0` or `1`", name)) - .with_annotation(Annotation::primary(item.value.span())), + Issue::error("The `strict_types` directive must be set to either `0` or `1`.") + .with_annotation( + Annotation::primary(item.value.span()) + .with_message("Invalid value assigned to the directive."), + ) + .with_note("The `strict_types` directive controls strict type enforcement and only accepts `0` (disabled) or `1` (enabled).") + .with_help("Set the directive value to either `0` or `1`."), ); } @@ -1854,39 +2024,59 @@ impl Walker> for SemanticsWalker { let parent = context.get_ancestor(context.get_ancestors_len() - 2); context.report( - Issue::error("strict types declaration must be at the top level") - .with_annotation(Annotation::primary(declare.span())) + Issue::error("The `strict_types` directive must be declared at the top level.") + .with_annotation( + Annotation::primary(declare.span()).with_message("Directive declared here."), + ) .with_annotation( Annotation::secondary(parent) - .with_message("this statement should come after the strict type declaration"), - ), + .with_message("This statement should follow the `strict_types` directive."), + ) + .with_help("Move the `strict_types` declaration to the top level of the file."), ); } } TICKS_DECLARE_DIRECTIVE => { if !matches!(item.value, Expression::Literal(Literal::Integer(_))) { context.report( - Issue::error(format!("`{}` declare directive must be set to a literal integer", name)) - .with_annotation(Annotation::primary(item.value.span())), + Issue::error("The `ticks` directive must be set to a literal integer.") + .with_annotation( + Annotation::primary(item.value.span()) + .with_message("Invalid value assigned to the directive."), + ) + .with_note("The `ticks` directive requires a literal integer value to specify the tick interval.") + .with_help("Provide a literal integer value for the `ticks` directive."), ); } } ENCODING_DECLARE_DIRECTIVE => { if !matches!(item.value, Expression::Literal(Literal::String(_))) { context.report( - Issue::error(format!("`{}` declare directive must be set to a literal integer", name)) - .with_annotation(Annotation::primary(item.value.span())), + Issue::error("The `encoding` declare directive must be set to a literal integer") + .with_annotation( + Annotation::primary(item.value.span()) + .with_message("Invalid value assigned to the directive."), + ) + .with_note("The `encoding` directive requires a literal string value to specify the character encoding.") + .with_help("Provide a literal string value for the `encoding` directive."), ); } } _ => { context.report( Issue::error(format!( - "`{}` is not a supported declare directive, supported directives are: `{}`", + "`{}` is not a supported `declare` directive. Supported directives are: `{}`.", name, DECLARE_DIRECTIVES.join("`, `") )) - .with_annotation(Annotation::primary(item.name.span())), + .with_annotation( + Annotation::primary(item.name.span()).with_message("Unsupported directive used here."), + ) + .with_note("Only specific directives are allowed in `declare` statements.") + .with_help(format!( + "Use one of the supported directives: `{}`.", + DECLARE_DIRECTIVES.join("`, `") + )), ); } } @@ -1899,12 +2089,17 @@ impl Walker> for SemanticsWalker { let parent = context.get_ancestor(context.get_ancestors_len() - 2); context.report( - Issue::error("namespace declaration must be at the top level") - .with_annotation(Annotation::primary(namespace.span())) + Issue::error("Namespace declaration must be at the top level.") + .with_annotation( + Annotation::primary(namespace.span()) + .with_message("Namespace declared here."), + ) .with_annotation( Annotation::secondary(parent) - .with_message("this statement should come after the namespace declaration"), - ), + .with_message("This statement should come after the namespace declaration."), + ) + .with_note("Namespace declarations define the scope of the code and should always appear at the top level.") + .with_help("Move the namespace declaration to the top level of the file."), ); } } @@ -1916,9 +2111,17 @@ impl Walker> for SemanticsWalker { let val = context.lookup_hint(&parenthesized_hint.hint); context.report( - Issue::error(format!("type `{}` cannot be parenthesized", val)) - .with_annotation(Annotation::primary(parenthesized_hint.hint.span())) - .with_annotation(Annotation::secondary(parenthesized_hint.span())), + Issue::error(format!("Type `{}` cannot be parenthesized.", val)) + .with_annotation( + Annotation::primary(parenthesized_hint.hint.span()) + .with_message("Invalid parenthesized type."), + ) + .with_annotation( + Annotation::secondary(parenthesized_hint.span()) + .with_message("Parenthesized type defined here."), + ) + .with_note("Only union or intersection types can be enclosed in parentheses.") + .with_help("Remove the parentheses around the type."), ); } } @@ -1927,9 +2130,14 @@ impl Walker> for SemanticsWalker { let val = context.lookup_hint(&nullable_hint.hint); context.report( - Issue::error(format!("type `{}` cannot be nullable", val)) - .with_annotation(Annotation::primary(nullable_hint.hint.span())) - .with_annotation(Annotation::secondary(nullable_hint.span())), + Issue::error(format!("Type `{}` cannot be nullable.", val)) + .with_annotation( + Annotation::primary(nullable_hint.hint.span()).with_message("Invalid nullable type."), + ) + .with_annotation( + Annotation::secondary(nullable_hint.span()).with_message("Nullable type defined here."), + ) + .with_help("Replace the type or remove the nullable modifier."), ); } } @@ -1938,9 +2146,15 @@ impl Walker> for SemanticsWalker { let val = context.lookup_hint(&union_hint.left); context.report( - Issue::error(format!("type `{}` cannot be part of a union", val)) - .with_annotation(Annotation::primary(union_hint.left.span())) - .with_annotation(Annotation::secondary(union_hint.pipe)), + Issue::error(format!("Type `{}` cannot be part of a union.", val)) + .with_annotation( + Annotation::primary(union_hint.left.span()).with_message("Invalid union type."), + ) + .with_annotation( + Annotation::secondary(union_hint.pipe).with_message("Union operator `|` used here."), + ) + .with_note("Intersection and standalone types cannot be part of a union.") + .with_help("Replace the type or remove it from the union."), ); } @@ -1948,19 +2162,34 @@ impl Walker> for SemanticsWalker { let val = context.lookup_hint(&union_hint.right); context.report( - Issue::error(format!("type `{}` cannot be part of a union", val)) - .with_annotation(Annotation::primary(union_hint.right.span())) - .with_annotation(Annotation::secondary(union_hint.pipe)), + Issue::error(format!("Type `{}` cannot be part of a union.", val)) + .with_annotation( + Annotation::primary(union_hint.right.span()).with_message("Invalid union type."), + ) + .with_annotation( + Annotation::secondary(union_hint.pipe).with_message("Union operator `|` used here."), + ) + .with_note("Intersection and standalone types cannot be part of a union.") + .with_help("Replace the type or remove it from the union."), ); } } Hint::Intersection(intersection_hint) => { if !intersection_hint.left.is_intersectable() { let val = context.lookup_hint(&intersection_hint.left); + context.report( - Issue::error(format!("type `{}` cannot be part of an intersection", val)) - .with_annotation(Annotation::primary(intersection_hint.left.span())) - .with_annotation(Annotation::secondary(intersection_hint.ampersand)), + Issue::error(format!("Type `{}` cannot be part of an intersection.", val)) + .with_annotation( + Annotation::primary(intersection_hint.left.span()) + .with_message("Invalid intersection type."), + ) + .with_annotation( + Annotation::secondary(intersection_hint.ampersand) + .with_message("Intersection operator `&` used here."), + ) + .with_note("Union and standalone types cannot be part of an intersection.") + .with_help("Replace the type or remove it from the intersection."), ); } @@ -1968,9 +2197,17 @@ impl Walker> for SemanticsWalker { let val = context.lookup_hint(&intersection_hint.right); context.report( - Issue::error(format!("type `{}` cannot be part of an intersection", val)) - .with_annotation(Annotation::primary(intersection_hint.right.span())) - .with_annotation(Annotation::secondary(intersection_hint.ampersand)), + Issue::error(format!("Type `{}` cannot be part of an intersection.", val)) + .with_annotation( + Annotation::primary(intersection_hint.right.span()) + .with_message("Invalid intersection type."), + ) + .with_annotation( + Annotation::secondary(intersection_hint.ampersand) + .with_message("Intersection operator `&` used here."), + ) + .with_note("Union and standalone types cannot be part of an intersection.") + .with_help("Replace the type or remove it from the intersection."), ); } } @@ -1981,13 +2218,12 @@ impl Walker> for SemanticsWalker { fn walk_in_try(&self, r#try: &Try, context: &mut Context<'_>) { if r#try.catch_clauses.is_empty() && r#try.finally_clause.is_none() { context.report( - Issue::error("cannot use `try` without a `catch` or `finally`") - .with_annotations([ - Annotation::primary(r#try.r#try.span()), - Annotation::secondary(r#try.block.span()), - ]) - .with_note("each `try` must have at least one corresponding `catch` or `finally` clause.") - .with_help("add either a `catch` or `finally` clause") + Issue::error("Cannot use `try` without a `catch` or `finally` clause.") + .with_annotation( + Annotation::primary(r#try.span()).with_message("`try` statement without `catch` or `finally`."), + ) + .with_note("Each `try` block must have at least one corresponding `catch` or `finally` clause.") + .with_help("Add either a `catch` or `finally` clause to the `try` block.") .with_link("https://www.php.net/manual/en/language.exceptions.php"), ); } @@ -2011,11 +2247,16 @@ impl Walker> for SemanticsWalker { for parameter in method.parameters.parameters.iter() { if parameter.is_promoted_property() { context.report( - Issue::error("promoted properties are not allowed in abstract constructors") - .with_annotation(Annotation::primary(parameter.span())) + Issue::error("Promoted properties are not allowed in abstract constructors.") + .with_annotation( + Annotation::primary(parameter.span()).with_message("Promoted property used here."), + ) .with_annotation( Annotation::secondary(abstract_modifier.span()) - .with_message("this constructor is abstract"), + .with_message("This constructor is abstract."), + ) + .with_help( + "Remove the promoted property from the constructor or make the constructor concrete.", ), ); } @@ -2033,12 +2274,16 @@ impl Walker> for SemanticsWalker { .any(|keyword| keyword.eq_ignore_ascii_case(class_name)) { context.report( - Issue::error(format!("class `{}` name cannot be a reserved keyword", class_name)) - .with_annotation(Annotation::primary(class.name.span())) + Issue::error(format!("Class `{}` name cannot be a reserved keyword.", class_name)) + .with_annotation( + Annotation::primary(class.name.span()) + .with_message(format!("Class name `{}` conflicts with a reserved keyword.", class_name)), + ) .with_annotation( Annotation::secondary(class.span()) - .with_message(format!("class `{}` defined here", class_fqcn)), - ), + .with_message(format!("Class `{}` declared here.", class_fqcn)), + ) + .with_help("Rename the class to avoid using reserved keywords."), ); } @@ -2048,61 +2293,71 @@ impl Walker> for SemanticsWalker { for modifier in class.modifiers.iter() { match &modifier { - Modifier::Static(_) | Modifier::PrivateSet(_) => { + Modifier::Static(_) => { context.report( - Issue::error(format!( - "class `{}` cannot have `{}` modifier", - class_name, - modifier.as_str(context.interner) - )) - .with_annotation(Annotation::primary(modifier.span())) - .with_annotation( - Annotation::secondary(class.span()) - .with_message(format!("class `{}` defined here", class_fqcn)), - ) - .with_help("remove the `static` modifier"), + Issue::error(format!("Class `{}` cannot have the `static` modifier.", class_name)) + .with_annotation( + Annotation::primary(modifier.span()).with_message("`static` modifier applied here."), + ) + .with_annotation( + Annotation::secondary(class.span()) + .with_message(format!("Class `{}` declared here.", class_fqcn)), + ) + .with_help("Remove the `static` modifier."), ); } - Modifier::Public(keyword) | Modifier::Protected(keyword) | Modifier::Private(keyword) => { + Modifier::Public(keyword) + | Modifier::Protected(keyword) + | Modifier::Private(keyword) + | Modifier::PrivateSet(keyword) => { let visibility_name = context.interner.lookup(&keyword.value); context.report( Issue::error(format!( - "class `{}` cannot have `{}` visibility modifier", + "Class `{}` cannot have the `{}` visibility modifier.", class_name, visibility_name )) - .with_annotation(Annotation::primary(keyword.span())) + .with_annotation( + Annotation::primary(keyword.span()) + .with_message(format!("`{}` modifier applied here.", visibility_name)), + ) .with_annotation( Annotation::secondary(class.span()) - .with_message(format!("class `{}` defined here", class_fqcn)), + .with_message(format!("Class `{}` declared here.", class_fqcn)), ) - .with_help(format!("remove the `{}` modifier", visibility_name)), + .with_help(format!("Remove the `{}` modifier.", visibility_name)), ); } Modifier::Final(keyword) => { if let Some(span) = last_abstract { context.report( - Issue::error(format!("abstract class `{}` cannot have `final` modifier", class_name)) - .with_annotation(Annotation::primary(keyword.span())) + Issue::error(format!("Abstract class `{}` cannot have the `final` modifier.", class_name)) + .with_annotation( + Annotation::primary(keyword.span()).with_message("`final` modifier applied here."), + ) .with_annotations([ - Annotation::secondary(span).with_message("previous `abstract` modifier"), + Annotation::secondary(span) + .with_message("Previous `abstract` modifier applied here."), Annotation::secondary(class.span()) - .with_message(format!("class `{}` defined here", class_fqcn)), + .with_message(format!("Class `{}` declared here.", class_fqcn)), ]) - .with_help("remove the `final` modifier"), + .with_help("Remove the `final` modifier from the abstract class."), ); } if let Some(span) = last_final { context.report( - Issue::error(format!("class `{}` cannot have multiple `final` modifiers", class_name)) - .with_annotation(Annotation::primary(keyword.span())) + Issue::error(format!("Class `{}` cannot have multiple `final` modifiers.", class_name)) + .with_annotation( + Annotation::primary(keyword.span()) + .with_message("Duplicate `final` modifier applied here."), + ) .with_annotations([ - Annotation::secondary(span).with_message("previous `final` modifier"), + Annotation::secondary(span).with_message("Previous `final` modifier applied here."), Annotation::secondary(class.span()) - .with_message(format!("class `{}` defined here", class_fqcn)), + .with_message(format!("Class `{}` declared here.", class_fqcn)), ]) - .with_help("remove the duplicate `final` modifier"), + .with_help("Remove the duplicate `final` modifier."), ); } @@ -2111,27 +2366,34 @@ impl Walker> for SemanticsWalker { Modifier::Abstract(keyword) => { if let Some(span) = last_final { context.report( - Issue::error(format!("final class `{}` cannot have `abstract` modifier", class_name)) - .with_annotation(Annotation::primary(keyword.span())) + Issue::error(format!("Final class `{}` cannot have the `abstract` modifier.", class_name)) + .with_annotation( + Annotation::primary(keyword.span()) + .with_message("`abstract` modifier applied here."), + ) .with_annotations([ - Annotation::secondary(span).with_message("previous `final` modifier"), + Annotation::secondary(span).with_message("Previous `final` modifier applied here."), Annotation::secondary(class.span()) - .with_message(format!("class `{}` defined here", class_fqcn)), + .with_message(format!("Class `{}` declared here.", class_fqcn)), ]) - .with_help("remove the `abstract` modifier"), + .with_help("Remove the `abstract` modifier from the final class."), ); } if let Some(span) = last_abstract { context.report( - Issue::error(format!("class `{}` cannot have multiple `abstract` modifiers", class_name)) - .with_annotation(Annotation::primary(keyword.span())) + Issue::error(format!("Class `{}` cannot have multiple `abstract` modifiers.", class_name)) + .with_annotation( + Annotation::primary(keyword.span()) + .with_message("Duplicate `abstract` modifier applied here."), + ) .with_annotations([ - Annotation::secondary(span).with_message("previous `abstract` modifier"), + Annotation::secondary(span) + .with_message("Previous `abstract` modifier applied here."), Annotation::secondary(class.span()) - .with_message(format!("class `{}` defined here", class_fqcn)), + .with_message(format!("Class `{}` declared here.", class_fqcn)), ]) - .with_help("remove the duplicate `abstract` modifier"), + .with_help("Remove the duplicate `abstract` modifier."), ); } @@ -2140,14 +2402,18 @@ impl Walker> for SemanticsWalker { Modifier::Readonly(keyword) => { if let Some(span) = last_readonly { context.report( - Issue::error(format!("class `{}` cannot have multiple `readonly` modifiers", class_name)) - .with_annotation(Annotation::primary(keyword.span())) + Issue::error(format!("Class `{}` cannot have multiple `readonly` modifiers.", class_name)) + .with_annotation( + Annotation::primary(keyword.span()) + .with_message("Duplicate `readonly` modifier applied here."), + ) .with_annotations([ - Annotation::secondary(span).with_message("previous `readonly` modifier"), + Annotation::secondary(span) + .with_message("Previous `readonly` modifier applied here."), Annotation::secondary(class.span()) - .with_message(format!("class `{}` defined here", class_fqcn)), + .with_message(format!("Class `{}` declared here.", class_fqcn)), ]) - .with_help("remove the duplicate `readonly` modifier"), + .with_help("Remove the duplicate `readonly` modifier."), ); } @@ -2170,12 +2436,13 @@ impl Walker> for SemanticsWalker { match &memeber { ClassLikeMember::EnumCase(case) => { context.report( - Issue::error(format!("class `{}` cannot contain enum cases", class_name)) - .with_annotation(Annotation::primary(case.span())) + Issue::error(format!("Class `{}` cannot contain enum cases.", class_name)) + .with_annotation(Annotation::primary(case.span()).with_message("Enum case found in class.")) .with_annotation( Annotation::secondary(class.span()) - .with_message(format!("class `{}` defined here", class_fqcn)), - ), + .with_message(format!("Class `{}` declared here.", class_fqcn)), + ) + .with_help("Remove the enum cases from the class definition."), ); } ClassLikeMember::Method(method) => { @@ -2184,16 +2451,18 @@ impl Walker> for SemanticsWalker { if !class.modifiers.contains_abstract() && method.modifiers.contains_abstract() { context.report( Issue::error(format!( - "class `{}` has an abstract method `{}`, and therefore must be declared abstract", + "Class `{}` contains an abstract method `{}`, so the class must be declared abstract.", class_name, method_name )) - .with_annotation(Annotation::primary(class.name.span())) .with_annotation( - Annotation::secondary(method.span()).with_message(format!( - "abstract method `{}::{}` is defined here", - class_name, method_name - )), - ), + Annotation::primary(class.name.span()) + .with_message("Class is missing the `abstract` modifier."), + ) + .with_annotation(Annotation::secondary(method.span()).with_message(format!( + "Abstract method `{}::{}` declared here.", + class_name, method_name + ))) + .with_help("Add the `abstract` modifier to the class."), ); } @@ -2229,12 +2498,16 @@ impl Walker> for SemanticsWalker { .any(|keyword| keyword.eq_ignore_ascii_case(interface_name)) { context.report( - Issue::error(format!("interface `{}` name cannot be a reserved keyword", interface_name)) - .with_annotation(Annotation::primary(interface.name.span())) + Issue::error(format!("Interface `{}` name cannot be a reserved keyword.", interface_name)) + .with_annotation( + Annotation::primary(interface.name.span()) + .with_message(format!("Interface `{}` declared here.", interface_name)), + ) .with_annotation( Annotation::secondary(interface.span()) - .with_message(format!("interface `{}` defined here", interface_fqcn)), - ), + .with_message(format!("Interface `{}` defined here.", interface_fqcn)), + ) + .with_help("Rename the interface to avoid using a reserved keyword."), ); } @@ -2263,22 +2536,27 @@ impl Walker> for SemanticsWalker { match &memeber { ClassLikeMember::TraitUse(trait_use) => { context.report( - Issue::error(format!("interface `{}` cannot use traits", interface_name)) - .with_annotation(Annotation::primary(trait_use.span())) + Issue::error(format!("Interface `{}` cannot use traits.", interface_name)) + .with_annotation(Annotation::primary(trait_use.span()).with_message("Trait use statement.")) .with_annotation( Annotation::secondary(interface.span()) - .with_message(format!("interface `{}` defined here", interface_fqcn)), - ), + .with_message(format!("Interface `{}` declared here.", interface_fqcn)), + ) + .with_help("Remove the trait use statement."), ); } ClassLikeMember::EnumCase(case) => { context.report( - Issue::error(format!("interface `{}` cannot contain enum cases", interface_name)) - .with_annotation(Annotation::primary(case.span())) + Issue::error(format!("Interface `{}` cannot contain enum cases.", interface_name)) + .with_annotation( + Annotation::primary(case.span()) + .with_message("Enum case declared here."), + ) .with_annotation( Annotation::secondary(interface.span()) - .with_message(format!("interface `{}` defined here", interface_fqcn)), - ), + .with_message(format!("Interface `{}` declared here.", interface_fqcn)), + ) + .with_note("Consider moving the enum case to an enum or class if it represents state or constants."), ); } ClassLikeMember::Method(method) => { @@ -2297,47 +2575,62 @@ impl Walker> for SemanticsWalker { context.report( Issue::error(format!( - "interface method `{}::{}` cannot have `{}` modifier", + "Interface method `{}::{}` cannot have `{}` modifier.", interface_name, method_name, visibility_name )) - .with_annotation(Annotation::primary(visibility.span())) + .with_annotation( + Annotation::primary(visibility.span()) + .with_message(format!("`{}` modifier applied here.", visibility_name)), + ) .with_annotation( Annotation::secondary(interface.span()) - .with_message(format!("`{}` defined here", interface_fqcn)), + .with_message(format!("Interface `{}` declared here.", interface_fqcn)), ) - .with_help(format!("remove the `{}` modifier", visibility_name)), + .with_help(format!( + "Remove the `{}` modifier from the method definition as methods in interfaces must always be public.", + visibility_name + )) + .with_note("Interface methods are always public and cannot have non-public visibility modifiers."), ); } if let MethodBody::Concrete(body) = &method.body { context.report( Issue::error(format!( - "interface method `{}::{}` cannot have a body", + "Interface method `{}::{}` cannot have a body.", interface_name, method_name )) .with_annotations([ - Annotation::primary(body.span()), - Annotation::primary(method.name.span()), + Annotation::primary(body.span()).with_message("Method body declared here."), + Annotation::primary(method.name.span()).with_message("Method name defined here."), Annotation::secondary(interface.span()) - .with_message(format!("`interface {}` defined here", interface_fqcn)), + .with_message(format!("Interface `{}` declared here.", interface_fqcn)), ]) - .with_help("replace the method body with a `;`"), + .with_help("Replace the method body with a `;` to indicate it is abstract.") + .with_note("Methods in interfaces cannot have implementations and must be abstract."), ); } if let Some(abstract_modifier) = method.modifiers.get_abstract() { context.report( Issue::error(format!( - "interface method `{}::{}` must not be abstract", + "Interface method `{}::{}` must not be abstract.", interface_name, method_name )) - .with_annotation(Annotation::primary(abstract_modifier.span())) + .with_annotation( + Annotation::primary(abstract_modifier.span()) + .with_message("Abstract modifier applied here."), + ) .with_annotations([ Annotation::secondary(interface.span()) - .with_message(format!("interface `{}` is defined here", interface_fqcn)), + .with_message(format!("Interface `{}` declared here.", interface_fqcn)), Annotation::secondary(method.span()) - .with_message(format!("method `{}::{}` defined here", interface_name, method_name)), - ]), + .with_message(format!("Method `{}::{}` declared here.", interface_name, method_name)), + ]) + .with_help("Remove the `abstract` modifier as all interface methods are implicitly abstract.") + .with_note( + "Adding the `abstract` modifier to an interface method is redundant because all interface methods are implicitly abstract.", + ), ); } @@ -2357,14 +2650,19 @@ impl Walker> for SemanticsWalker { Property::Plain(plain_property) => { context.report( Issue::error(format!( - "interface `{}` cannot have non-hooked properties", + "Interface `{}` cannot have non-hooked properties.", interface_name )) - .with_annotation(Annotation::primary(plain_property.span())) + .with_annotation( + Annotation::primary(plain_property.span()) + .with_message("Non-hooked property declared here."), + ) .with_annotation( Annotation::secondary(interface.span()) - .with_message(format!("`{}` defined here", interface_fqcn)), - ), + .with_message(format!("Interface `{}` declared here.", interface_fqcn)), + ) + .with_note("Interfaces are intended to define behavior and cannot include concrete property declarations.") + .with_help("Remove the non-hooked property from the interface or convert it into a hooked property.") ); } Property::Hooked(hooked_property) => { @@ -2393,79 +2691,107 @@ impl Walker> for SemanticsWalker { context.report( Issue::error(format!( - "interface virtual property `{}::{}` must not specify asymmetric visibility", + "Interface virtual property `{}::{}` must not specify asymmetric visibility.", interface_name, property_name, )) - .with_annotation(Annotation::primary(visibility.span())) + .with_annotation( + Annotation::primary(visibility.span()) + .with_message(format!("Asymmetric visibility modifier `{}` applied here.", visibility_name)), + ) .with_annotation( Annotation::secondary(interface.span()) - .with_message(format!("`{}` defined here", interface_fqcn)), + .with_message(format!("Interface `{}` defined here.", interface_fqcn)), ) - .with_help(format!("remove the `{}` modifier", visibility_name)), + .with_help(format!( + "Remove the `{}` modifier from the property to make it compatible with interface constraints.", + visibility_name + )), ); } for visibility in non_public_read_visibilities { let visibility_name = visibility.as_str(context.interner); + context.report( Issue::error(format!( - "interface virtual property `{}::{}` cannot have `{}` modifier", + "Interface virtual property `{}::{}` cannot have `{}` modifier.", interface_name, property_name, visibility_name, )) - .with_annotation(Annotation::primary(visibility.span())) + .with_annotation(Annotation::primary(visibility.span()).with_message(format!( + "Visibility modifier `{}` applied here.", + visibility_name + ))) .with_annotation( Annotation::secondary(interface.span()) - .with_message(format!("`{}` defined here", interface_fqcn)), + .with_message(format!("Interface `{}` defined here.", interface_fqcn)), ) - .with_help(format!("remove the `{}` modifier", visibility_name)), + .with_help(format!( + "Remove the `{}` modifier from the property to meet interface requirements.", + visibility_name + )), ); } if !found_public { context.report( Issue::error(format!( - "interface property `{}::{}` must be declared public", + "Interface virtual property `{}::{}` must be declared public.", interface_name, property_name )) - .with_annotation(Annotation::primary(hooked_property.item.variable().span())) + .with_annotation( + Annotation::primary(hooked_property.span()) + .with_message("Property defined here."), + ) .with_annotation( Annotation::secondary(interface.span()) - .with_message(format!("`{}` defined here", interface_fqcn)), + .with_message(format!("Interface `{}` defined here.", interface_fqcn)), ) - .with_help("add the `public` visibility modifier to the property"), + .with_help("Add the `public` visibility modifier to the property."), ); } if let Some(abstract_modifier) = hooked_property.modifiers.get_abstract() { context.report( - Issue::error(format!( - "interface property `{}::{}` cannot be abstract", - interface_name, property_name - )) - .with_annotation(Annotation::primary(abstract_modifier.span())) - .with_annotations([ - Annotation::secondary(hooked_property.item.variable().span()), - Annotation::secondary(interface.span()).with_message(format!("`{}` defined here", interface_fqcn)), - ]) - .with_note( - "all interface properties are implicitly abstract, and cannot be explicitly abstract.", - ), - ); + Issue::error(format!( + "Interface virtual property `{}::{}` cannot be abstract.", + interface_name, property_name + )) + .with_annotation( + Annotation::primary(abstract_modifier.span()) + .with_message("Abstract modifier applied here."), + ) + .with_annotations([ + Annotation::secondary(hooked_property.span()) + .with_message("Property defined here."), + Annotation::secondary(interface.span()) + .with_message(format!("Interface `{}` defined here.", interface_fqcn)), + ]) + .with_note( + "All interface virtual properties are implicitly abstract and cannot be explicitly declared as abstract.", + ), + ); } if let PropertyItem::Concrete(item) = &hooked_property.item { context.report( Issue::error(format!( - "interface hooked property `{}::{}` cannot have a default value", + "Interface virtual property `{}::{}` cannot have a default value.", interface_name, property_name )) - .with_annotation(Annotation::primary(item.span())) + .with_annotation( + Annotation::primary(item.equals.join(item.value.span())) + .with_message("Default value assigned here."), + ) + .with_annotation( + Annotation::secondary(hooked_property.span()) + .with_message("Property defined here.") + ) .with_annotation( Annotation::secondary(interface.span()) - .with_message(format!("`{}` defined here", interface_fqcn)), + .with_message(format!("Interface `{}` defined here.", interface_fqcn)), ) .with_note( - "interface properties are virtual properties which cannot contain a default value", + "Interface properties are virtual properties and cannot contain a default value.", ), ); } @@ -2474,16 +2800,22 @@ impl Walker> for SemanticsWalker { if let PropertyHookBody::Concrete(property_hook_concrete_body) = &hook.body { context.report( Issue::error(format!( - "interface hooked property `{}::{}` must be abstract", + "Interface virtual property `{}::{}` must be abstract.", interface_name, property_name )) - .with_annotation(Annotation::primary(property_hook_concrete_body.span())) - .with_annotations([ - Annotation::primary(hooked_property.item.variable().span()), + .with_annotation( + Annotation::primary(property_hook_concrete_body.span()) + .with_message("Body defined here."), + ) + .with_annotation( + Annotation::secondary(hooked_property.item.variable().span()) + .with_message("Property declared here."), + ) + .with_annotation( Annotation::secondary(interface.span()) - .with_message(format!("`{}` defined here", interface_fqcn)), - ]) - .with_note("abstract hooked properties do not contain a body"), + .with_message(format!("Interface `{}` defined here.", interface_fqcn)), + ) + .with_note("Abstract hooked properties must not contain a body."), ); } } @@ -2513,15 +2845,20 @@ impl Walker> for SemanticsWalker { context.report( Issue::error(format!( - "interface constant cannot have `{}` visibility modifier", + "Interface constant cannot have `{}` visibility modifier.", visibility_name, )) - .with_annotation(Annotation::primary(visibility.span())) .with_annotation( - Annotation::secondary(interface.span()) - .with_message(format!("`{}` defined here", interface_fqcn)), + Annotation::primary(visibility.span()) + .with_message(format!("Visibility modifier `{}` applied here.", visibility_name)), ) - .with_help(format!("remove the `{}` modifier", visibility_name)), + .with_help(format!( + "Remove the `{}` modifier from the constant to comply with interface requirements.", + visibility_name + )) + .with_note( + "Interface constants are implicitly public and cannot have a non-public visibility modifier.", + ), ); } @@ -2548,12 +2885,16 @@ impl Walker> for SemanticsWalker { .any(|keyword| keyword.eq_ignore_ascii_case(class_like_name)) { context.report( - Issue::error(format!("trait `{}` name cannot be a reserved keyword", class_like_name)) - .with_annotation(Annotation::primary(r#trait.name.span())) + Issue::error(format!("Trait `{}` name cannot be a reserved keyword.", class_like_name)) + .with_annotation( + Annotation::primary(r#trait.name.span()) + .with_message(format!("Trait `{}` declared here.", class_like_name)), + ) .with_annotation( Annotation::secondary(r#trait.span()) - .with_message(format!("trait `{}` defined here", class_like_fqcn)), - ), + .with_message(format!("Trait `{}` defined here.", class_like_fqcn)), + ) + .with_help("Rename the trait to a non-reserved keyword."), ); } @@ -2563,12 +2904,13 @@ impl Walker> for SemanticsWalker { match &member { ClassLikeMember::EnumCase(case) => { context.report( - Issue::error(format!("trait `{}` cannot contain enum cases", class_like_name)) - .with_annotation(Annotation::primary(case.span())) + Issue::error(format!("Trait `{}` cannot contain enum cases.", class_like_name)) + .with_annotation(Annotation::primary(case.span()).with_message("Enum case defined here.")) .with_annotation( Annotation::secondary(r#trait.span()) - .with_message(format!("trait `{}` defined here", class_like_fqcn)), - ), + .with_message(format!("Trait `{}` defined here.", class_like_fqcn)), + ) + .with_help("Remove the enum case from the trait."), ); } ClassLikeMember::Method(method) => { @@ -2620,11 +2962,16 @@ impl Walker> for SemanticsWalker { || SOFT_RESERVED_KEYWORDS_MINUS_SYMBOL_ALLOWED.iter().any(|keyword| keyword.eq_ignore_ascii_case(enum_name)) { context.report( - Issue::error(format!("enum `{}` name cannot be a reserved keyword", enum_name)) - .with_annotation(Annotation::primary(r#enum.name.span())) + Issue::error(format!("Enum `{}` name cannot be a reserved keyword.", enum_name)) .with_annotation( - Annotation::secondary(r#enum.span()).with_message(format!("enum `{}` defined here", enum_fqcn)), - ), + Annotation::primary(r#enum.name.span()) + .with_message(format!("Reserved keyword used as the enum name `{}`.", enum_name)), + ) + .with_annotation( + Annotation::secondary(r#enum.span()) + .with_message(format!("Enum `{}` defined here.", enum_fqcn)), + ) + .with_help(format!("Rename the enum `{}` to a non-reserved keyword.", enum_name)), ); } @@ -2634,14 +2981,18 @@ impl Walker> for SemanticsWalker { context.report( Issue::error(format!( - "enum `{}` backing type must be either `string` or `int`, found `{}`.", + "Enum `{}` backing type must be either `string` or `int`, but found `{}`.", enum_name, key )) - .with_annotation(Annotation::primary(hint.span())) + .with_annotation( + Annotation::primary(hint.span()) + .with_message(format!("Invalid backing type `{}` specified here.", key)), + ) .with_annotation( Annotation::secondary(r#enum.name.span()) - .with_message(format!("enum `{}` defined here", enum_fqcn)), - ), + .with_message(format!("Enum `{}` defined here.", enum_fqcn)), + ) + .with_help("Change the backing type to either `string` or `int`."), ); } } @@ -2652,8 +3003,8 @@ impl Walker> for SemanticsWalker { self.process_members(&r#enum.members, r#enum.span(), enum_name, enum_fqcn, "enum", context); - for memeber in r#enum.members.iter() { - match &memeber { + for member in r#enum.members.iter() { + match &member { ClassLikeMember::EnumCase(case) => { let item_name_id = case.item.name().value; let item_name = context.interner.lookup(&item_name_id); @@ -2663,14 +3014,21 @@ impl Walker> for SemanticsWalker { if enum_is_backed { context.report( Issue::error(format!( - "case `{}` of backed enum `{}` must have a value", + "Case `{}` of backed enum `{}` must have a value.", item_name, enum_name )) - .with_annotation(Annotation::primary(case.span())) + .with_annotation( + Annotation::primary(case.span()) + .with_message(format!("Case `{}` defined here.", item_name)), + ) .with_annotation( Annotation::secondary(r#enum.span()) - .with_message(format!("enum `{}` defined here", enum_fqcn)), - ), + .with_message(format!("Enum `{}` defined here.", enum_fqcn)), + ) + .with_help(format!( + "Add a value to case `{}` or remove the backing from the enum `{}`.", + item_name, enum_name + )), ); } } @@ -2678,16 +3036,25 @@ impl Walker> for SemanticsWalker { if !enum_is_backed { context.report( Issue::error(format!( - "case `{}` of unbacked enum `{}` must not have a value", + "Case `{}` of unbacked enum `{}` must not have a value.", item_name, enum_name )) - .with_annotation(Annotation::primary(item.equals.span().join(item.value.span()))) + .with_annotation( + Annotation::primary(item.equals.span().join(item.value.span())) + .with_message("Value assigned to the enum case."), + ) .with_annotations([ - Annotation::secondary(item.name.span()) - .with_message(format!("case `{}::{}` defined here", enum_name, item_name)), + Annotation::secondary(item.name.span()).with_message(format!( + "Case `{}::{}` declared here.", + enum_name, item_name + )), Annotation::secondary(r#enum.span()) - .with_message(format!("enum `{}` defined here", enum_fqcn)), - ]), + .with_message(format!("Enum `{}` defined here.", enum_fqcn)), + ]) + .with_help(format!( + "Remove the value from case `{}` or make the enum `{}` backed.", + item_name, enum_name + )), ); } } @@ -2702,24 +3069,41 @@ impl Walker> for SemanticsWalker { { context.report( Issue::error(format!( - "enum `{}` cannot contain magic method `{}`", + "Enum `{}` cannot contain magic method `{}`.", enum_name, magic_method )) - .with_annotation(Annotation::primary(method.name.span)) - .with_annotation(Annotation::secondary(r#enum.name.span())), + .with_annotation( + Annotation::primary(method.name.span) + .with_message(format!("Magic method `{}` declared here.", method_name)), + ) + .with_annotation( + Annotation::secondary(r#enum.name.span()) + .with_message(format!("Enum `{}` declared here.", enum_fqcn)), + ) + .with_help(format!( + "Remove the magic method `{}` from the enum `{}`.", + method_name, enum_name + )), ); } if let Some(abstract_modifier) = method.modifiers.get_abstract() { context.report( - Issue::error(format!("enum method `{}::{}` must not be abstract", enum_name, method_name)) - .with_annotation(Annotation::primary(abstract_modifier.span())) + Issue::error(format!("Enum method `{}::{}` must not be abstract.", enum_name, method_name)) + .with_annotation( + Annotation::primary(abstract_modifier.span()) + .with_message("Abstract modifier found here."), + ) .with_annotations([ Annotation::secondary(r#enum.span()) - .with_message(format!("enum `{}` is defined here", enum_fqcn)), + .with_message(format!("Enum `{}` defined here.", enum_fqcn)), Annotation::secondary(method.span()) - .with_message(format!("method `{}::{}` defined here", enum_name, method_name)), - ]), + .with_message(format!("Method `{}::{}` defined here.", enum_name, method_name)), + ]) + .with_help(format!( + "Remove the abstract modifier from the method `{}` in enum `{}`.", + method_name, enum_name + )), ); } @@ -2736,9 +3120,15 @@ impl Walker> for SemanticsWalker { } ClassLikeMember::Property(property) => { context.report( - Issue::error(format!("enum `{}` cannot have properties", enum_name)) - .with_annotation(Annotation::primary(property.span())) - .with_annotation(Annotation::secondary(r#enum.span())), + Issue::error(format!("Enum `{}` cannot have properties.", enum_name)) + .with_annotation( + Annotation::primary(property.span()).with_message("Property defined here."), + ) + .with_annotation( + Annotation::secondary(r#enum.span()) + .with_message(format!("Enum `{}` defined here.", enum_fqcn)), + ) + .with_help(format!("Remove the property from the enum `{}`.", enum_name)), ); self.process_property(property, r#enum.span(), "enum", enum_name, enum_fqcn, false, context); @@ -2774,31 +3164,39 @@ impl Walker> for SemanticsWalker { context.report( Issue::error(format!( - "class `{}` cannot have `{}` modifier", + "Anonymous class `{}` cannot have the `{}` modifier.", ANONYMOUS_CLASS_NAME, modifier_name )) - .with_annotation(Annotation::primary(modifier.span())) + .with_annotation( + Annotation::primary(modifier.span()) + .with_message(format!("`{}` modifier applied here.", modifier_name)), + ) .with_annotation( Annotation::secondary(anonymous_class.span()) - .with_message(format!("class `{}` defined here", ANONYMOUS_CLASS_NAME)), + .with_message(format!("Anonymous class `{}` defined here.", ANONYMOUS_CLASS_NAME)), ) - .with_help(format!("help: remove the `{}` modifier", modifier_name)), + .with_help(format!("Remove the `{}` modifier from the class definition.", modifier_name)), ); } Modifier::Final(keyword) => { if let Some(span) = last_final { context.report( Issue::error(format!( - "class `{}` cannot have multiple `final` modifiers", + "Anonymous class `{}` cannot have multiple `final` modifiers.", ANONYMOUS_CLASS_NAME )) - .with_annotation(Annotation::primary(keyword.span())) - .with_annotation(Annotation::secondary(span).with_message("previous `final` modifier")) + .with_annotation( + Annotation::primary(keyword.span()) + .with_message("Duplicate `final` modifier applied here."), + ) + .with_annotation( + Annotation::secondary(span).with_message("Previous `final` modifier applied here."), + ) .with_annotation( Annotation::secondary(anonymous_class.span()) - .with_message(format!("class `{}` defined here", ANONYMOUS_CLASS_NAME)), + .with_message(format!("Anonymous class `{}` defined here.", ANONYMOUS_CLASS_NAME)), ) - .with_help("help: remove the duplicate `final` modifier"), + .with_help("Remove the duplicate `final` modifier."), ); } @@ -2808,16 +3206,17 @@ impl Walker> for SemanticsWalker { if let Some(span) = last_readonly { context.report( Issue::error(format!( - "class `{}` cannot have multiple `readonly` modifiers", + "Anonymous class `{}` cannot have multiple `readonly` modifiers.", ANONYMOUS_CLASS_NAME )) .with_annotations([ - Annotation::primary(keyword.span), - Annotation::secondary(span).with_message("previous `readonly` modifier"), + Annotation::primary(keyword.span) + .with_message("Duplicate `readonly` modifier applied here."), + Annotation::secondary(span).with_message("Previous `readonly` modifier applied here."), Annotation::secondary(anonymous_class.span()) - .with_message(format!("class `{}` defined here", ANONYMOUS_CLASS_NAME)), + .with_message(format!("Anonymous class `{}` defined here.", ANONYMOUS_CLASS_NAME)), ]) - .with_help("help: remove the duplicate `readonly` modifier"), + .with_help("Remove the duplicate `readonly` modifier."), ); } @@ -2863,12 +3262,13 @@ impl Walker> for SemanticsWalker { match &member { ClassLikeMember::EnumCase(case) => { context.report( - Issue::error(format!("class `{}` cannot contain enum cases", ANONYMOUS_CLASS_NAME)) + Issue::error(format!("Anonymous class `{}` cannot contain enum cases.", ANONYMOUS_CLASS_NAME)) .with_annotations([ - Annotation::primary(case.span()), + Annotation::primary(case.span()).with_message("Enum case defined here."), Annotation::secondary(anonymous_class.span()) - .with_message(format!("class `{}` defined here", ANONYMOUS_CLASS_NAME)), - ]), + .with_message(format!("Anonymous class `{}` defined here.", ANONYMOUS_CLASS_NAME)), + ]) + .with_help("Remove the enum case from the anonymous class definition."), ); } ClassLikeMember::Method(method) => { @@ -2876,14 +3276,19 @@ impl Walker> for SemanticsWalker { if let Some(abstract_modifier) = method.modifiers.get_abstract() { context.report( - Issue::error(format!("anonymous class method `{}` must not be abstract", method_name)) - .with_annotations([ - Annotation::primary(abstract_modifier.span()), - Annotation::secondary(anonymous_class.span()) - .with_message(format!("class `{}` defined here", ANONYMOUS_CLASS_NAME)), - Annotation::secondary(method.span()) - .with_message(format!("method `{}` defined here", method_name)), - ]), + Issue::error(format!( + "Method `{}` in anonymous class `{}` must not be abstract.", + method_name, ANONYMOUS_CLASS_NAME + )) + .with_annotations([ + Annotation::primary(abstract_modifier.span()) + .with_message("Abstract modifier applied here."), + Annotation::secondary(anonymous_class.span()) + .with_message(format!("Anonymous class `{}` defined here.", ANONYMOUS_CLASS_NAME)), + Annotation::secondary(method.span()) + .with_message(format!("Method `{}` defined here.", method_name)), + ]) + .with_help("Remove the `abstract` modifier from the method."), ); } @@ -2944,15 +3349,15 @@ impl Walker> for SemanticsWalker { if let Some(val) = &r#return.value { context.report( Issue::error(format!( - "function `{}` with return type of `void` must not return a value", + "Function `{}` with return type `void` must not return a value.", name )) - .with_annotation(Annotation::primary(val.span())) + .with_annotation(Annotation::primary(val.span()).with_message("Return value found here.")) .with_annotation( Annotation::secondary(function.span()) - .with_message(format!("function `{}` defined here", fqfn)), + .with_message(format!("Function `{}` defined here.", fqfn)), ) - .with_help("help: remove the return type hint, or remove the return value"), + .with_help("Remove the return type hint or the return value."), ); } } @@ -2960,13 +3365,15 @@ impl Walker> for SemanticsWalker { Hint::Never(_) => { for r#return in returns { context.report( - Issue::error(format!("function `{}` with return type of `never` must not return", name)) - .with_annotation(Annotation::primary(r#return.span())) + Issue::error(format!("Function `{}` with return type `never` must not return.", name)) + .with_annotation( + Annotation::primary(r#return.span()).with_message("Return statement found here."), + ) .with_annotation( Annotation::secondary(function.span()) - .with_message(format!("function `{}` defined here", fqfn)), + .with_message(format!("Function `{}` defined here.", fqfn)), ) - .with_help("remove the return type hint, or remove the return statement"), + .with_help("Remove the return type hint or the return statement."), ); } } @@ -2974,14 +3381,17 @@ impl Walker> for SemanticsWalker { for r#return in returns { if r#return.value.is_none() { context.report( - Issue::error(format!("function `{}` with return type must return a value", name)) - .with_annotation(Annotation::primary(r#return.span())) + Issue::error(format!("Function `{}` with a return type must return a value.", name)) + .with_annotation( + Annotation::primary(r#return.span()) + .with_message("Empty return statement found here."), + ) .with_annotation( Annotation::secondary(function.span()) - .with_message(format!("function `{}` defined here", fqfn)), + .with_message(format!("Function `{}` defined here.", fqfn)), ) - .with_note("did you mean `return null;` instead of `return;`?") - .with_help("add a return value to the statement"), + .with_note("Did you mean `return null;` instead of `return;`?") + .with_help("Add a return value to the statement."), ); } } @@ -2992,7 +3402,6 @@ impl Walker> for SemanticsWalker { fn walk_in_attribute(&self, attribute: &Attribute, context: &mut Context<'_>) { let name = context.interner.lookup(&attribute.name.value()); - if let Some(list) = &attribute.arguments { for argument in list.arguments.iter() { let (ellipsis, value) = match &argument { @@ -3004,25 +3413,27 @@ impl Walker> for SemanticsWalker { if let Some(ellipsis) = ellipsis { context.report( - Issue::error("cannot use argument unpacking in attribute arguments") - .with_annotation(Annotation::primary(ellipsis.span())) + Issue::error("Cannot use argument unpacking in attribute arguments.") + .with_annotation( + Annotation::primary(ellipsis.span()).with_message("Argument unpacking used here."), + ) .with_annotation( Annotation::secondary(attribute.name.span()) - .with_message(format!("attribute `{}` defined here", name)), + .with_message(format!("Attribute `{}` defined here.", name)), ) - .with_note("unpacking arguments is not allowed in attribute arguments"), + .with_note("Unpacking arguments is not allowed in attribute arguments."), ); } if !value.is_constant(true) { context.report( - Issue::error(format!("attribute `{}` argument contains a non-constant expression", name)) + Issue::error(format!("Attribute `{}` argument contains a non-constant expression.", name)) .with_annotations([ - Annotation::primary(value.span()), + Annotation::primary(value.span()).with_message("Non-constant expression used here."), Annotation::secondary(attribute.name.span()) - .with_message(format!("attribute `{}` defined here", name)), + .with_message(format!("Attribute `{}` defined here.", name)), ]) - .with_note("attribute arguments must be constant expressions"), + .with_note("Attribute arguments must be constant expressions."), ); } } @@ -3037,11 +3448,12 @@ impl Walker> for SemanticsWalker { return; } - // If we reach this point, the label was not found - // try to see if there is a label with the same name but different case - // if so, suggest the correct label + // If we reach this point, the label was not found. + // Attempt to find a label with the same name but different case. + // If found, suggest the correct label. let going_to = context.interner.lookup(&goto.label.value); let mut suggestions = vec![]; + for label in all_labels { let label_name = context.interner.lookup(&label.name.value); if label_name.eq_ignore_ascii_case(going_to) { @@ -3050,19 +3462,23 @@ impl Walker> for SemanticsWalker { } let mut issue = - Issue::error(format!("undefined goto label `{}`", going_to)) - .with_annotation(Annotation::primary(goto.label.span)) + Issue::error(format!("Undefined `goto` label `{}`.", going_to)) + .with_annotation(Annotation::primary(goto.label.span).with_message("This `goto` label is not defined.")) .with_annotations(suggestions.iter().map(|(name, span)| { - Annotation::secondary(*span).with_message(format!("did you mean `{}`?", name)) + Annotation::secondary(*span).with_message(format!("Did you mean `{}`?", name)) })); - if 1 == suggestions.len() { - issue = - issue.with_note(format!("goto label `{}` not found, did you mean `{}`?", going_to, suggestions[0].0)); + if suggestions.len() == 1 { + issue = issue.with_note(format!( + "The `goto` label `{}` was not found. Did you mean `{}`?", + going_to, suggestions[0].0 + )); } else if !suggestions.is_empty() { let names = suggestions.iter().map(|(name, _)| format!("`{}`", name)).collect::>().join(", "); - - issue = issue.with_note(format!("goto label `{}` not found, did you mean one of: {}?", going_to, names)); + issue = issue.with_note(format!( + "The `goto` label `{}` was not found. Did you mean one of the following: {}?", + going_to, names + )); } context.report(issue); @@ -3078,12 +3494,14 @@ impl Walker> for SemanticsWalker { if let Some(ellipsis) = positional_argument.ellipsis { if let Some(last_named_argument) = last_named_argument { context.report( - Issue::error("cannot use argument unpacking after a named argument") - .with_annotation(Annotation::primary(ellipsis.span())) + Issue::error("Cannot use argument unpacking after a named argument.") .with_annotation( - Annotation::secondary(last_named_argument).with_message("named argument here"), + Annotation::primary(ellipsis.span()).with_message("Unpacking argument here."), ) - .with_note("unpacking arguments must come before named arguments"), + .with_annotation( + Annotation::secondary(last_named_argument).with_message("Named argument here."), + ) + .with_note("Unpacking arguments must come before named arguments."), ); } @@ -3091,23 +3509,29 @@ impl Walker> for SemanticsWalker { } else { if let Some(named_argument) = last_named_argument { context.report( - Issue::error("cannot use positional argument after a named argument") - .with_annotation(Annotation::primary(positional_argument.span())) + Issue::error("Cannot use positional argument after a named argument.") + .with_annotation( + Annotation::primary(positional_argument.span()) + .with_message("Positional argument defined here."), + ) .with_annotation( - Annotation::secondary(named_argument).with_message("named argument here"), + Annotation::secondary(named_argument).with_message("Named argument here."), ) - .with_note("positional arguments must come before named arguments"), + .with_note("Positional arguments must come before named arguments."), ); } if let Some(unpacking) = last_unpacking { context.report( - Issue::error("cannot use positional argument after argument unpacking") - .with_annotation(Annotation::primary(positional_argument.span())) + Issue::error("Cannot use positional argument after argument unpacking.") .with_annotation( - Annotation::secondary(unpacking).with_message("argument unpacking here"), + Annotation::primary(positional_argument.span()) + .with_message("Positional argument defined here."), ) - .with_note("positional arguments must come before unpacking arguments"), + .with_annotation( + Annotation::secondary(unpacking).with_message("Argument unpacking here."), + ) + .with_note("Positional arguments must come before unpacking arguments."), ); } } @@ -3115,12 +3539,16 @@ impl Walker> for SemanticsWalker { Argument::Named(named_argument) => { if let Some(ellipsis) = named_argument.ellipsis { context.report( - Issue::error("cannot use argument unpacking in named arguments") - .with_annotation(Annotation::primary(ellipsis.span())) + Issue::error("Cannot use argument unpacking in named arguments.") + .with_annotation( + Annotation::primary(ellipsis.span()) + .with_message("Unpacking argument defined here."), + ) .with_annotation( - Annotation::secondary(named_argument.span()).with_message("named argument here"), + Annotation::secondary(named_argument.span()) + .with_message("Named argument defined here."), ) - .with_note("unpacking arguments is not allowed in named arguments"), + .with_note("Unpacking arguments is not allowed in named arguments."), ); } @@ -3146,12 +3574,17 @@ impl Walker> for SemanticsWalker { for r#return in returns { if let Some(val) = &r#return.value { context.report( - Issue::error("closure with return type of `void` must not return a value") - .with_annotation(Annotation::primary(val.span())) + Issue::error("Closure with a return type of `void` must not return a value.") .with_annotation( - Annotation::secondary(closure.function.span).with_message("closure defined here"), + Annotation::primary(val.span()) + .with_message("This value is not allowed with a `void` return type."), ) - .with_help("remove the return type hint, or remove the return value"), + .with_annotation( + Annotation::secondary(closure.span()).with_message("Closure defined here."), + ) + .with_help( + "Remove the return value, or change the return type hint to an appropriate type.", + ), ); } } @@ -3159,12 +3592,17 @@ impl Walker> for SemanticsWalker { Hint::Never(_) => { for r#return in returns { context.report( - Issue::error("closure with return type of `never` must not return") - .with_annotation(Annotation::primary(r#return.span())) + Issue::error("Closure with a return type of `never` must not include a return statement.") .with_annotation( - Annotation::secondary(closure.function.span).with_message("closure defined here"), + Annotation::primary(r#return.span()) + .with_message("Return statement is not allowed with a `never` return type."), ) - .with_help("remove the return type hint, or remove the return statement"), + .with_annotation( + Annotation::secondary(closure.span()).with_message("Closure defined here."), + ) + .with_help( + "Remove the return statement, or change the return type hint to a compatible type.", + ), ); } } @@ -3172,13 +3610,15 @@ impl Walker> for SemanticsWalker { for r#return in returns { if r#return.value.is_none() { context.report( - Issue::error("closure with return type must return a value") - .with_annotation(Annotation::primary(r#return.span())) + Issue::error("Closure with a return type must return a value.") .with_annotation( - Annotation::secondary(closure.function.span).with_message("closure defined here"), + Annotation::primary(r#return.span()).with_message("Missing return value."), ) - .with_note("did you mean `return null;` instead of `return;`?") - .with_help("add a return value to the statement"), + .with_annotation( + Annotation::secondary(closure.span()).with_message("Closure defined here."), + ) + .with_note("Did you mean `return null;` instead of `return;`?") + .with_help("Add a return value that matches the expected return type."), ); } } @@ -3198,10 +3638,16 @@ impl Walker> for SemanticsWalker { // see: https://3v4l.org/VgoiO if let Hint::Void(_) = &return_hint.hint { context.report( - Issue::error("arrow function cannot have a return type of `void`") - .with_annotation(Annotation::primary(return_hint.hint.span())) - .with_annotation(Annotation::secondary(arrow_function.r#fn.span)) - .with_help("remove the return type hint, or use a different type"), + Issue::error("Arrow function cannot have a return type of `void`.") + .with_annotation( + Annotation::primary(return_hint.hint.span()) + .with_message("Return type `void` is not valid for an arrow function."), + ) + .with_annotation( + Annotation::secondary(arrow_function.r#fn.span) + .with_message("Arrow function defined here."), + ) + .with_help("Remove the `void` return type hint, or replace it with a valid type."), ); } } @@ -3221,9 +3667,15 @@ impl Walker> for SemanticsWalker { parameters_seen.iter().find_map(|(n, s)| if parameter.variable.name.eq(n) { Some(s) } else { None }) { context.report( - Issue::error(format!("parameter `{}` is already defined", name)) - .with_annotation(Annotation::primary(parameter.variable.span())) - .with_annotation(Annotation::secondary(*prev_span).with_message("previously defined here")), + Issue::error(format!("Parameter `{}` is already defined.", name)) + .with_annotation( + Annotation::primary(parameter.variable.span()) + .with_message("This parameter is redefined here."), + ) + .with_annotation( + Annotation::secondary(*prev_span).with_message("The original parameter was defined here."), + ) + .with_help("Ensure all parameter names are unique within the parameter list."), ); } else if !parameter.is_promoted_property() { parameters_seen.push((parameter.variable.name, parameter.variable.span())); @@ -3237,27 +3689,36 @@ impl Walker> for SemanticsWalker { Modifier::Static(keyword) | Modifier::Final(keyword) | Modifier::Abstract(keyword) => { context.report( Issue::error(format!( - "parameter `{}` cannot have modifier `{}`", + "Parameter `{}` cannot have the `{}` modifier.", name, context.interner.lookup(&keyword.value) )) - .with_annotation(Annotation::primary(modifier.span())) + .with_annotation(Annotation::primary(modifier.span()).with_message(format!( + "Invalid `{}` modifier used here.", + context.interner.lookup(&keyword.value) + ))) .with_annotation( Annotation::secondary(parameter.variable.span) - .with_message(format!("parameter `{}` defined here", name)), + .with_message(format!("Parameter `{}` defined here.", name)), ) - .with_help("remove the modifier from the parameter"), + .with_help("Remove the invalid modifier from the parameter."), ); } Modifier::Readonly(_) => { if let Some(s) = last_readonly { context.report( - Issue::error(format!("parameter `{}` cannot have multiple `readonly` modifiers", name)) - .with_annotation(Annotation::primary(modifier.span())) - .with_annotation( - Annotation::secondary(s).with_message("previous `readonly` modifier"), - ) - .with_help("remove the duplicate `readonly` modifier"), + Issue::error(format!( + "Parameter `{}` cannot have multiple `readonly` modifiers.", + name + )) + .with_annotation( + Annotation::primary(modifier.span()) + .with_message("Duplicate `readonly` modifier used here."), + ) + .with_annotation( + Annotation::secondary(s).with_message("Previous `readonly` modifier used here."), + ) + .with_help("Remove the duplicate `readonly` modifier."), ); } else { last_readonly = Some(modifier.span()); @@ -3266,12 +3727,18 @@ impl Walker> for SemanticsWalker { Modifier::Public(_) | Modifier::Protected(_) | Modifier::Private(_) => { if let Some(s) = last_read_visibility { context.report( - Issue::error(format!("parameter `{}` cannot have multiple visibility modifiers", name)) - .with_annotation(Annotation::primary(modifier.span())) - .with_annotation( - Annotation::secondary(s).with_message("previous visibility modifier"), - ) - .with_help("remove the duplicate visibility modifier"), + Issue::error(format!( + "Parameter `{}` cannot have multiple visibility modifiers.", + name + )) + .with_annotation( + Annotation::primary(modifier.span()) + .with_message("Duplicate visibility modifier used here."), + ) + .with_annotation( + Annotation::secondary(s).with_message("Previous visibility modifier used here."), + ) + .with_help("Remove the duplicate visibility modifier."), ); } else { last_read_visibility = Some(modifier.span()); @@ -3281,12 +3748,18 @@ impl Walker> for SemanticsWalker { if let Some(s) = last_write_visibility { context.report( Issue::error(format!( - "parameter `{}` cannot have multiple write visibility modifiers", + "Parameter `{}` cannot have multiple write visibility modifiers.", name )) - .with_annotation(Annotation::primary(modifier.span())) - .with_annotation(Annotation::secondary(s).with_message("previous visibility modifier")) - .with_help("remove the duplicate visibility modifier"), + .with_annotation( + Annotation::primary(modifier.span()) + .with_message("Duplicate write visibility modifier used here."), + ) + .with_annotation( + Annotation::secondary(s) + .with_message("Previous write visibility modifier used here."), + ) + .with_help("Remove the duplicate write visibility modifier."), ); } else { last_write_visibility = Some(modifier.span()); @@ -3298,26 +3771,44 @@ impl Walker> for SemanticsWalker { if let Some((n, s)) = last_variadic { context.report( Issue::error(format!( - "parameter `{}` is defined after variadic parameter `{}`", + "Invalid parameter order: parameter `{}` is defined after variadic parameter `{}`.", name, context.interner.lookup(&n) )) - .with_annotation(Annotation::primary(parameter.variable.span())) - .with_annotation(Annotation::secondary(s).with_message("variadic parameter defined here")) - .with_help("move all parameters after the variadic parameter to the end of the parameter list"), + .with_annotation( + Annotation::primary(parameter.variable.span()) + .with_message(format!("Parameter `{}` is defined here.", name)), + ) + .with_annotation( + Annotation::secondary(s).with_message(format!( + "Variadic parameter `{}` is defined here.", + context.interner.lookup(&n) + )), + ) + .with_help( + "Move all parameters following the variadic parameter to the end of the parameter list.", + ), ); } if let Some(ellipsis) = parameter.ellipsis { if let Some(default) = ¶meter.default_value { context.report( - Issue::error(format!("variadic parameter `{}` cannot have a default value", name)) - .with_annotation(Annotation::primary(default.span())) - .with_annotation( - Annotation::secondary(ellipsis.join(parameter.variable.span)) - .with_message(format!("parameter `{}` is variadic", name)), - ) - .with_help("remove the default value from the variadic parameter"), + Issue::error(format!( + "Invalid parameter definition: variadic parameter `{}` cannot have a default value.", + name + )) + .with_annotation( + Annotation::primary(default.span()).with_message(format!( + "Default value is defined for variadic parameter `{}` here.", + name + )), + ) + .with_annotation( + Annotation::secondary(ellipsis.join(parameter.variable.span)) + .with_message(format!("Parameter `{}` is variadic and marked with `...` here.", name)), + ) + .with_help("Remove the default value from the variadic parameter."), ); } @@ -3330,12 +3821,19 @@ impl Walker> for SemanticsWalker { let hint_name = context.lookup_hint(hint); context.report( - Issue::error(format!("bottom type `{}` cannot be used as a parameter type", hint_name)) - .with_annotation(Annotation::primary(hint.span())) - .with_annotation( - Annotation::secondary(parameter.variable.span()) - .with_message(format!("parameter `{}` defined here", name)), - ), + Issue::error(format!( + "Invalid parameter type: bottom type `{}` cannot be used as a parameter type.", + hint_name + )) + .with_annotation( + Annotation::primary(hint.span()) + .with_message(format!("Bottom type `{}` is not allowed here.", hint_name)), + ) + .with_annotation( + Annotation::secondary(parameter.variable.span()) + .with_message(format!("This parameter `{}` is defined here.", name)), + ) + .with_help("Use a valid parameter type to ensure compatibility with PHP's type system."), ); } } @@ -3349,13 +3847,18 @@ impl Walker> for SemanticsWalker { if let MatchArm::Default(default_arm) = &arm { if let Some(previous) = last_default { context.report( - Issue::error("match expression may only contain one default arm") - .with_annotation(Annotation::primary(default_arm.span())) + Issue::error("A match expression can only have one default arm.") + .with_annotation( + Annotation::primary(default_arm.span()) + .with_message("This is a duplicate default arm."), + ) + .with_annotation( + Annotation::secondary(previous).with_message("The first default arm is defined here."), + ) .with_annotation( - Annotation::secondary(previous).with_message("previous default arm defined here"), + Annotation::secondary(r#match.span()).with_message("Match expression defined here."), ) - .with_annotation(Annotation::secondary(r#match.span())) - .with_help("remove this default case"), + .with_help("Remove this duplicate default arm to ensure the match expression is valid."), ); } else { last_default = Some(default_arm.default.span); @@ -3367,22 +3870,21 @@ impl Walker> for SemanticsWalker { fn walk_in_switch(&self, switch: &Switch, context: &mut Context<'_>) { let mut last_default: Option = None; - let cases = match &switch.body { - SwitchBody::BraceDelimited(switch_brace_delimited_body) => &switch_brace_delimited_body.cases, - SwitchBody::ColonDelimited(switch_colon_delimited_body) => &switch_colon_delimited_body.cases, - }; - - for case in cases.iter() { + for case in switch.body.cases() { if let SwitchCase::Default(default_case) = &case { if let Some(previous) = last_default { context.report( - Issue::error("switch statement may only contain one default case") - .with_annotation(Annotation::primary(default_case.span())) + Issue::error("A switch statement can only have one default case.") + .with_annotation( + Annotation::primary(default_case.span()).with_message("This is a duplicate default case."), + ) + .with_annotation( + Annotation::secondary(previous).with_message("The first default case is defined here."), + ) .with_annotation( - Annotation::secondary(previous).with_message("previous default case defined here"), + Annotation::secondary(switch.span()).with_message("Switch statement containing the duplicate cases."), ) - .with_annotation(Annotation::secondary(switch.span())) - .with_help("remove this default case"), + .with_help("Remove this duplicate default case to ensure the switch statement is valid and unambiguous."), ); } else { last_default = Some(default_case.default.span); diff --git a/crates/symbol-table/src/symbol.rs b/crates/symbol-table/src/symbol.rs index 71f0e64..1fc9464 100644 --- a/crates/symbol-table/src/symbol.rs +++ b/crates/symbol-table/src/symbol.rs @@ -10,7 +10,6 @@ use mago_span::Span; /// Represents the different kinds of symbols that can be encountered in the code. #[derive(Debug, Clone, Copy, Eq, PartialEq, Hash, Serialize, Deserialize, PartialOrd, Ord, Display)] -#[strum(serialize_all = "kebab-case")] pub enum SymbolKind { /// A class definition. Class,