[SwiftDiagnostics] Make GroupedDiagnostics Emit Notes#3279
[SwiftDiagnostics] Make GroupedDiagnostics Emit Notes#3279filip-sakel wants to merge 7 commits intoswiftlang:mainfrom
GroupedDiagnostics Emit Notes#3279Conversation
|
Hi @ahoppen, I hope you're well! I'm following up as I'll have some free time in the next few days to work on this code. Please let me know if you’d like me to start reviewing it so we can get the PR merged. |
|
Could you add a test where there are multiple error diagnostics anchored at the same location? As well as multiple notes? |
…add more test cases.
|
Sorry for the delay; here are the changes I made. For one, I generalized the test name to "testGroupingForSameLineDiagnostics" to better match the rest of the tests for group diagnostics. Now, we test a variety of things:
Please let me know if you'd like additional tests or if you think I should break up this test function into smaller, separate functions. |
|
Kind reminder for @ahoppen, @kathygray-pl or anyone else who can review :) (Also sorry for closing and reopening the PR, the GitHub website on mobile is a bit unwieldy.) |
The PR addresses this issue. Originally, #2214 tried tackling this, but it got split into several PRs (including #2238 which I heavily lean on in this PR). Since #2214 was based on a 2024 version of the project, I opted to just fork
maininstead of that PR's branch.I'm not sure if there's a cleaner approach here, but I just modified
DiagnosticFormatterto allow including notes in the output.DiagnosticFormatteralready handles multiple diagnostics per line, so I just modified it to print “annotations” (an array of diagnostics and notes) instead of just diagnostics.Also, I added a simple test with notes at different nesting levels, and one multi-line diagnostic. Of course, I can add more tests if we want to be more thorough.
Finally, you’ll notice some auxiliary changes in
DiagnosticFormatter.annotatedSourcesthat aren't technically necessary:annotationsPerLinewith dictionaries: The previous code iterated over the source lines and filtered the diagnostics every iteration, resulting in quadratic time complexity. I just rewrote this code using dictionaries. This approach simplifies adding “annotations” (which are either diagnostics or notes), and is slightly more efficient. As a result, the I was able to convert the for-loop that createsannotatedSourceLinesto anArray.map.Please let me know if I should try a different approach altogether, revert some of the auxiliary changes, or add more tests.