Skip to content

Commit

Permalink
chore: improve all issue messages
Browse files Browse the repository at this point in the history
Signed-off-by: azjezz <azjezz@protonmail.com>
  • Loading branch information
azjezz committed Dec 15, 2024
1 parent 986b587 commit 0ca9a7f
Show file tree
Hide file tree
Showing 66 changed files with 1,686 additions and 1,172 deletions.
90 changes: 45 additions & 45 deletions crates/docblock/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.")
}
}
}
Expand All @@ -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.",
}
}
}
8 changes: 4 additions & 4 deletions crates/lexer/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}

Expand All @@ -43,6 +43,6 @@ impl From<SyntaxError> 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."))
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -34,11 +34,11 @@ impl<'a> Walker<LintContext<'a>> 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.
Expand Down
133 changes: 57 additions & 76 deletions crates/linter/src/plugin/best_practices/rules/disallowed_functions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<Level> {
Some(Level::Warning)
}
}

fn report_disallowed_extension_function(&self, function_call: &FunctionCall, context: &mut LintContext) {
impl<'a> Walker<LintContext<'a>> 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<Level> {
Some(Level::Warning)
}
}

impl<'a> Walker<LintContext<'a>> 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);
}
}
Loading

0 comments on commit 0ca9a7f

Please sign in to comment.