-
Notifications
You must be signed in to change notification settings - Fork 192
[JENKINS-73509] GCC parser does not handle cc1/cc1plus warnings #1253
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
d95815f to
569f920
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks almost good now, thanks for your patience...
| @Override | ||
| protected Optional<Issue> createIssue(final Matcher matcher, final LookaheadStream lookahead, | ||
| final IssueBuilder builder) { | ||
| // Extract named groups from the regex match |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do not comment things that are expressed as code.
| // Extract named groups from the regex match |
| String severityLevel = matcher.group("severity"); // warning, error, or note | ||
| String messageContent = matcher.group("message"); // The actual message text | ||
|
|
||
| // Validate that all required fields are present |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| // Validate that all required fields are present |
| /** | ||
| * Maps the severity string from compiler output to a Severity enum value. | ||
| * | ||
| * Mapping: | ||
| * - "warning" → WARNING_NORMAL | ||
| * - "error" → WARNING_HIGH | ||
| * - "note" → WARNING_LOW | ||
| * - default → WARNING_NORMAL | ||
| * | ||
| * @param severityString the severity string from the compiler output | ||
| * (warning/error/note) | ||
| * @return the corresponding Severity enum value for issue classification | ||
| */ | ||
| private Severity mapSeverity(final String severityString) { | ||
| if (equalsIgnoreCase(severityString, "warning")) { | ||
| return Severity.WARNING_NORMAL; | ||
| } else if (equalsIgnoreCase(severityString, "error")) { | ||
| return Severity.WARNING_HIGH; | ||
| } else if (equalsIgnoreCase(severityString, "note")) { | ||
| return Severity.WARNING_LOW; | ||
| } | ||
| return Severity.WARNING_NORMAL; // Default fallback | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Replace with Severity.guessSeverity
| private boolean isCc1MessageContinuation(final LookaheadStream lookahead) { | ||
| var peek = lookahead.peekNext(); | ||
| if (peek.length() < 3) { | ||
| return false; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can't we reuse the same method from the Gcc4Parser?
| */ | ||
| private static final String GCC_CC1_WARNING_PATTERN = "^(?:In .+?:\\s*)?" // Optional context prefix (e.g., "In member function:") | ||
| + "(?<compiler>cc1(?:plus)?):\\s*" // Compiler name: cc1 or cc1plus | ||
| + "(?<severity>warning|error|note):\\s*" // Severity level |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you override, this will speed up the parser:
@Override
protected boolean isLineInteresting(final String line) {
return line.contains("warning") || line.contains("error") || line.contains("note");
}
|
Hello @uhafner I have implemented all suggested changes. Please review the changes. |
|
Thanks! I squashed your commits, please make sure that you write meaningful messages the next time... |
|
Thanks! I’ll make sure to write clear and meaningful commit messages next time. |
JENKINS-73509
GCC parser does not handle cc1/cc1plus warnings
Testing done
Submitter checklist