Duplicated diagnostic causing crash on DefaultDiagnosticConsoleFormatter

[Kyle-Ye]

Description

See detail on #678 (comment)

Checklist

• If possible, I've reproduced the issue using the main branch of this package.
• This issue hasn't been addressed in an existing GitHub issue.

Expected Behavior

Only emit one diagnostic for the actual location - the Extension file.

Actual behavior

Emit 2 duplicated diagnostic: the Extension Markdown file and the source Swift file

Steps To Reproduce

Change eat(_:quantity:) to eat(_:quantity:extraLog:) and build documentation for SlothCreator.

Swift-DocC Version Information

5.9.0

Swift Compiler Version Information

swift-driver version: 1.87.1 Apple Swift version 5.9 (swiftlang-5.9.0.128.108 clang-1500.0.40.1)
Target: arm64-apple-macosx14.0

[d-ronnqvist]

I think that I have looked into this in the past, before it resulted in a crash. We are also tracking this issue as rdar://79991171. I'll associate that radar with this issue so that people who find the radar can find this issue instead.

I may have some old notes that may still be relevant. Let me see if I can find them.

[d-ronnqvist]

I couldn't find my old notes but I vaguely remember that I got this far...

It looks like the links is resolved for the full content but for both "chunks" (the in-source doc comment and the extensions code).

We need to process both chunks so that we can get diagnostics to the right places but I think that the ReferenceResolver shouldn't visit the full semantic both times.

I remember trying to change this and that it wasn't as easy as I thought but I don't remember what issues I encountered.

[Kyle-Ye]

Can we filter such duplicated diagnostic by checking the source url matching? Here is my draft PR #733

Before the PR:

Emit 2 duplicated diagnostic: the Extension Markdown file and the source Swift file

And then crash on DefaultDiagnosticConsoleFormatter

After the PR:

Only emit one diagnostic for the actual location - the Extension file.

And the DefaultDiagnosticConsoleFormatter will not crash

warning: 'eat(_:quantity:)' doesn't exist at '/SlothCreator/Sloth'
  --> Extensions/Sloth.md:12:5-12:21
10 | ### Engaging in Activities
11 | 
12 + - ``eat(_:quantity:)``
   |     ╰─suggestion: Replace 'eat(_:quantity:)' with 'eat(_:quantity:extraLog:)'
13 | - ``sleep(in:for:)``
14 | ========================================
Starting Local Preview Server
	 Address: http://localhost:8080/documentation/slothcreator
	          http://localhost:8080/tutorials/slothcreator

If you agree on this general idea of the PR, I'll continue to work on it to add the test case. @d-ronnqvist

[d-ronnqvist]

That's sounds like a good idea.

[Kyle-Ye]

The test case is added and the PR is ready for a review. @d-ronnqvist

[Kyle-Ye]

This issue is not fully resolved. Encounter same crash recently.

Background:
SwiftPackageIndex/SwiftPackageIndex-Server#3061 (comment)

I can reproduce the issue on both release/5.10 and main using the large repo code.

But when I try to prepare a minimal reproducible package. It only crash for Xcode 15.3 or release/5.10 branch of swift-docc not on main branch.

I guess we have fixed part of the issue again on some commits on main branch.

Found it, it is #840

I'll work on preparing a crash example case which will also crash on main branch of swift-docc later this week.

[Kyle-Ye]

A reproducible package

DemoKit.zip

swift package generate-documentation --verbose
# Copy `docc invocation` command and use the parameter to debug docc

It is still due to duplicated diagnostic problem (Emit the same issue in Demo.swift and Demo.md file)

[Kyle-Ye]

A quick fix for this issue is add similar logic to the following position or unresolvedResourceProblem. See #918

https://github.com/apple/swift-docc/blob/5029117adab75c36f75e3a90fe4e691969730c84/Sources/SwiftDocC/Semantics/MarkupReferenceResolver.swift#L102-L105

But it feels like this does not seem to be the right direction, we just enumerate the holes. Similar crashes are still possible in the future. cc @d-ronnqvist

[d-ronnqvist]

A quick fix for this issue is add similar logic to the following position or unresolvedResourceProblem. See #918

https://github.com/apple/swift-docc/blob/5029117adab75c36f75e3a90fe4e691969730c84/Sources/SwiftDocC/Semantics/MarkupReferenceResolver.swift#L102-L105

But it feels like this does not seem to be the right direction, we just enumerate the holes. Similar crashes are still possible in the future. cc @d-ronnqvist

I agree that fixing individual places that create out-of-bounds diagnostic ranges doesn't address the root cause.

As a first step to a long term fix we should make sure that the diagnostic formatter handles all out-of-bounds diagnostic ranges instead of assuming that they're valid. I opened #947 to do this.

After that, @patshaughnessy and I talked about possible changes to design away the need to do two MarkupReferenceResolver passes. If those ideas pan out it will remove these duplicate diagnostics.