Skip to content

feat(cli): add lint error types to problem output - #125

Open
devin-ai-integration[bot] wants to merge 2 commits into
mainfrom
devin/1785536447-lint-error-types
Open

feat(cli): add lint error types to problem output#125
devin-ai-integration[bot] wants to merge 2 commits into
mainfrom
devin/1785536447-lint-error-types

Conversation

@devin-ai-integration

Copy link
Copy Markdown

Summary

Closes #106: linter problems now carry a type instead of being plain strings, so clu lint output explains what kind of problem was found.

New common::problem module:

pub enum LintErrorType { File, Dir, Category, ChangeType, Description, PullRequest, Release, Whitespace, Duplicate, Other }

pub struct Problem { error_type: LintErrorType, path: PathBuf, line: Option<usize>, message: String }

LintErrorType::match_problem(&str) is the matcher deriving the type from the problem description (e.g. "PR link is not matching PR number ..." -> PullRequest, "duplicate release: ..." -> Duplicate, "'ABI' should be used instead of 'ABi'" -> Description). Problem::new calls it, so all existing check functions keep producing String descriptions and only the aggregation layer changes:

-pub fn add_to_problems(problems: &mut Vec<String>, fp, line, problem) { problems.push(format!("{fp}:{line}: {problem}")) }
+pub fn add_to_problems(problems: &mut Vec<Problem>, fp, line, problem) { problems.push(Problem::new(fp, line, problem)) }

SingleFileChangelog.problems, MultiFileChangelog.problems and Changelog::get_problems() are Vec<Problem> / &[Problem] now.

Output format gained the type and a per-type summary:

found problems in changelog:
CHANGELOG.md:11 [pr]: PR link is not matching PR number 1948: '.../pull/1949'
CHANGELOG.md:21 [description]: 'ABI' should be used instead of 'ABi'

problems by type:
  description: 3
  duplicate: 3
  pr: 1

Follow-up option (not done here to keep the diff focused): push Problem down into the leaf checks so each check states its type explicitly instead of relying on the string matcher.

Link to Devin session: https://app.devin.ai/sessions/155810153b9847b79dcff5c9d330cc97
Requested by: @MalteHerrmann

Co-Authored-By: malteherrmann.mail@gmail.com <malteherrmann.mail@gmail.com>
@MalteHerrmann MalteHerrmann self-assigned this Jul 31, 2026
@devin-ai-integration

Copy link
Copy Markdown
Author

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
  • Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

  • Disable automatic comment, CI, and merge conflict monitoring

Co-Authored-By: malteherrmann.mail@gmail.com <malteherrmann.mail@gmail.com>

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Devin Review found 1 potential issue.

Open in Devin Review

Comment thread src/common/problem.rs
Comment on lines +34 to +80
pub fn match_problem(problem: &str) -> LintErrorType {
let lowercase = problem.to_lowercase();

if lowercase.starts_with("duplicate ") {
return LintErrorType::Duplicate;
}

if lowercase.contains("filename") || lowercase.contains("found in file") {
return LintErrorType::File;
}

if lowercase.contains("directory") {
return LintErrorType::Dir;
}

if lowercase.contains("whitespace") || lowercase.contains(" space ") {
return LintErrorType::Whitespace;
}

if lowercase.contains("pr link") || lowercase.contains("pr number") {
return LintErrorType::PullRequest;
}

if lowercase.contains("change type") {
return LintErrorType::ChangeType;
}

if lowercase.contains("category") {
return LintErrorType::Category;
}

if lowercase.contains("description")
|| lowercase.contains("should be used instead of")
|| lowercase.contains("malformed entry")
{
return LintErrorType::Description;
}

if lowercase.contains("version string")
|| lowercase.contains("release link")
|| lowercase.contains("unreleased header")
{
return LintErrorType::Release;
}

LintErrorType::Other
}

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

🟡 Lint problems can be labelled with the wrong problem type when the changelog text happens to contain certain words

The problem type is guessed by searching the whole problem sentence, including the user's own changelog text that is quoted inside it (LintErrorType::match_problem at src/common/problem.rs:34-80), so an entry whose text mentions words like "directory", "whitespace" or "filename" is reported under a completely unrelated problem type.

Impact: Users see wrong labels next to some lint problems and misleading counts in the new per-type summary.

How the whole-message keyword search picks up quoted entry text

Messages such as "PR description should end with a dot: '{}'" (src/common/entry.rs:67) and "malformed entry: '{}'" (src/single_file/entry.rs:47) embed the raw changelog line. match_problem checks contains("directory"), contains("filename"), contains("whitespace"), contains(" space "), contains("pr link"), contains("change type") and contains("category") before the description branch, so e.g. PR description should end with a dot: 'Move directory handling' is classified as Dir, and ... 'Add category filters' as Category.

Separately, the change-type spelling problem "'{}' should be used instead of '{}'" produced at src/single_file/change_type.rs:70-73 is a change-type issue but matches the should be used instead of rule and is reported as description.

A more robust fix is the follow-up the description mentions: have each leaf check pass its own LintErrorType instead of deriving it from the rendered message, or at least match only on the fixed prefix of the message before the quoted user content.

Prompt for agents
LintErrorType::match_problem in src/common/problem.rs derives the lint error type by keyword-searching the entire rendered problem message. Many problem messages embed arbitrary user content (the offending changelog line or description), e.g. "PR description should end with a dot: '<desc>'" in src/common/entry.rs and "malformed entry: '<line>'" in src/single_file/entry.rs. Because keyword checks for directory/filename/whitespace/space/pr link/change type/category run before the description check, an entry whose text contains those words gets classified under the wrong type, corrupting both the per-problem label and the per-type summary printed by src/cli/lint.rs. Additionally, the change-type spelling message produced in src/single_file/change_type.rs ("'X' should be used instead of 'Y'") is classified as Description rather than ChangeType. Consider having the individual checks supply their LintErrorType explicitly (the follow-up mentioned in the PR description), or match only against the portion of the message preceding quoted user content.
Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

This is mentioned as a potential follow-up on this PR -- to push the problem type down to the leaves and avoid the pattern matching. I want this to be done on this PR because this pattern matching is not only ugly code but it's error-prone as well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add type of lint error to output

1 participant