Skip to content

fixed problemmatcher regexp#6490

Merged
sean-mcmanus merged 8 commits intomicrosoft:masterfrom
guntern:problemMatcherFix
Nov 17, 2020
Merged

fixed problemmatcher regexp#6490
sean-mcmanus merged 8 commits intomicrosoft:masterfrom
guntern:problemMatcherFix

Conversation

@guntern
Copy link
Copy Markdown
Contributor

@guntern guntern commented Nov 10, 2020

Some gcc versions do not print the column number of an error or warning. These will not be catched by the problem matcher. I tweaked the regular expression so that it also matches, when the column number is not present.
The change in the first group is necessary so that it does not expand because of the greedyness to include the line number. It could also be replaced by (.*?) to have lazy matching.

@sean-mcmanus
Copy link
Copy Markdown
Contributor

How do we repro gcc outputting warnings/errors without a column?

@bobbrow
Copy link
Copy Markdown
Member

bobbrow commented Nov 10, 2020

You can write a task that does an 'echo' of an error line without the column and set the problem matcher to this version of gcc.

@sean-mcmanus
Copy link
Copy Markdown
Contributor

You can write a task that does an 'echo' of an error line without the column and set the problem matcher to this version of gcc.

Yeah, if required for testing, but I wanted to know how to repro the issue with gcc, i.e. what does "some gcc versions" mean?

@guntern
Copy link
Copy Markdown
Contributor Author

guntern commented Nov 11, 2020

Sorry, for not mentioning the version.
The problem ocurred for us with a gcc 3.4.6. This version produces output with and without column numbers. Two examples below:

File.c:416:10: warning: #warning test
File.c:423: error: [12874] conflicting types for 'test'

I just exchanged the file names.

I tested the regular expression on regex101.com until it worked as intended.

According to https://stackoverflow.com/questions/54541148/how-to-define-a-problem-matcher-for-cppcheck-task-in-vscode you can also reproduce the error when using cppcheck 1.82.

Copy link
Copy Markdown
Contributor

@sean-mcmanus sean-mcmanus left a comment

Choose a reason for hiding this comment

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

[^:]* doesn't seems to work with Windows style file paths (e.g. c:\Users). This might be fixable via adding something like (.:)? before that. I think the original .* was not 100% guaranteed to work.

@sean-mcmanus sean-mcmanus added this to the 1.2.0 milestone Nov 14, 2020
@guntern
Copy link
Copy Markdown
Contributor Author

guntern commented Nov 14, 2020

Indeed, Windows style full paths did not work. So I extended my test cases to

File.c:416:10: warning: #warning test
File.c:423: error: [12874] conflicting types for 'test'
C:\Path\to\file.c:423: error: [12874] conflicting types for 'test'
C:\Path\to\file.c:416:10: error: [12874] conflicting types for 'test'

I now changed the first group to (.*?), which is now doing lazy matching. This also prevents the line number to become part of the filename, if the column number is missing.

@guntern guntern requested a review from sean-mcmanus November 14, 2020 18:54
@sean-mcmanus
Copy link
Copy Markdown
Contributor

C:\Path\to\file.c:416:10 error: [12874] conflicting types for 'test'

Is this test case valid (no colon after the comma) or should the regex (\d*):? be replaced with something like (?:(\d+):)??

@guntern
Copy link
Copy Markdown
Contributor Author

guntern commented Nov 17, 2020

C:\Path\to\file.c:416:10 error: [12874] conflicting types for 'test'

Is this test case valid (no colon after the comma) or should the regex (\d*):? be replaced with something like (?:(\d+):)??

Indeed, I missed the colon there. I updated the test cases in my comment accordingly.

However, my proposed version works with and without this colon. (?:(\d+):)? just matches when the colon is present. So it comes down to how strict the matching should be. Well there is one difference according to regex101: (\d*):? produces an empty group if it does not match. (?:(\d+):)? omits the group at all. I was afraid that this missing group would then cause errors in VS Code. But apparently it can handle this very well. So both versions work in VS Code and list the problems correctly.

So in my point of view both versions work, although slightly differently. It is up to you, which version you prefer.

@michelleangela
Copy link
Copy Markdown
Contributor

(\d*):? that creates an empty group if the column number is not there seems to be a better choice to avoid potential errors in VS Code.

@sean-mcmanus
Copy link
Copy Markdown
Contributor

(\d*):? that creates an empty group if the column number is not there seems to be a better choice to avoid potential errors in VS Code.

Yeah, that seems fine to me.

@sean-mcmanus sean-mcmanus merged commit e5d08d2 into microsoft:master Nov 17, 2020
@guntern guntern deleted the problemMatcherFix branch November 20, 2020 06:39
@github-actions github-actions Bot locked and limited conversation to collaborators Jan 2, 2021
@sean-mcmanus
Copy link
Copy Markdown
Contributor

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants