Skip to content

Conversation

@Kyle-Ye
Copy link
Contributor

@Kyle-Ye Kyle-Ye commented Oct 6, 2023

Bug/issue #, if applicable:

Close #729 and rdar://79991171

Summary

If the range’s source is not matched with the resolver’s source, we should not emit a problem

Dependencies

None

Testing

See func testDuplicatedDiagnosticForExtensionFile() in MarkupReferenceResolverTests.swift

Checklist

Make sure you check off the following items. If they cannot be completed, provide a reason.

  • Added tests
  • Ran the ./bin/test script and it succeeded
  • Updated documentation if necessary

@Kyle-Ye Kyle-Ye marked this pull request as ready for review October 6, 2023 05:11
@Kyle-Ye Kyle-Ye requested a review from d-ronnqvist October 6, 2023 05:11
@Kyle-Ye Kyle-Ye force-pushed the bugfix/duplicate_diag branch from b5fbd86 to cf59e07 Compare October 15, 2023 07:17
@Kyle-Ye
Copy link
Contributor Author

Kyle-Ye commented Oct 15, 2023

@swift-ci please test

@Kyle-Ye Kyle-Ye force-pushed the bugfix/duplicate_diag branch from cf59e07 to 80deede Compare October 15, 2023 07:28
@Kyle-Ye
Copy link
Contributor Author

Kyle-Ye commented Oct 16, 2023

@swift-ci please test

@d-ronnqvist
Copy link
Contributor

This looks good to me. @binamaniar can you take a second look at this while I'm out?

@Kyle-Ye
Copy link
Contributor Author

Kyle-Ye commented Oct 17, 2023

@swift-ci please test

@Kyle-Ye
Copy link
Contributor Author

Kyle-Ye commented Oct 23, 2023

Ping @binamaniar

@Kyle-Ye
Copy link
Contributor Author

Kyle-Ye commented Oct 24, 2023

This looks good to me. @binamaniar can you take a second look at this while I'm out?

@d-ronnqvist I'd like to confirm whether we should wait for binamaniar's approval before the merging.

@binamaniar
Copy link
Contributor

binamaniar commented Oct 24, 2023

sorry for the late reply. Looks good to me to make sure there is no crash when the location is not in source. Just thinking do we need to need an extra check that it is for extension files only. My answer is probably no but sharing my thought. I don't see any other scenario where we would run into this.

@Kyle-Ye
Copy link
Contributor Author

Kyle-Ye commented Oct 25, 2023

Just thinking do we need to need an extra check that it is for extension files only.

There is not too much context to get the info(whether it is an extension file) now.
[By checking the extension of the source is "md"? 🤔]

We may consider adding the extra check later if needed. Anyway, thanks for the review suggestion.

Kyle-Ye and others added 5 commits October 25, 2023 13:32
If the range’s source is not matched with the resolver’s source, we should not emit a problem
Co-authored-by: David Rönnqvist <david.ronnqvist@gmail.com>
Co-authored-by: David Rönnqvist <david.ronnqvist@gmail.com>
Co-authored-by: David Rönnqvist <david.ronnqvist@gmail.com>
@Kyle-Ye Kyle-Ye force-pushed the bugfix/duplicate_diag branch from 003e5c5 to b987502 Compare October 25, 2023 05:33
@Kyle-Ye Kyle-Ye linked an issue Oct 25, 2023 that may be closed by this pull request
2 tasks
@Kyle-Ye
Copy link
Contributor Author

Kyle-Ye commented Oct 25, 2023

@swift-ci please test

@Kyle-Ye Kyle-Ye merged commit 4e9d7f8 into swiftlang:main Oct 25, 2023
@Kyle-Ye Kyle-Ye deleted the bugfix/duplicate_diag branch October 25, 2023 06:15
Kyle-Ye added a commit to Kyle-Ye/swift-docc that referenced this pull request Oct 25, 2023
* Fix duplicated diagnostic issue

If the range’s source is not matched with the resolver’s source, we should not emit a problem

* Add test case for duplicated diagnostic issue

* Update Sources/SwiftDocC/Semantics/MarkupReferenceResolver.swift

Co-authored-by: David Rönnqvist <david.ronnqvist@gmail.com>

* Update Tests/SwiftDocCTests/Semantics/MarkupReferenceResolverTests.swift

Co-authored-by: David Rönnqvist <david.ronnqvist@gmail.com>

* Update Tests/SwiftDocCTests/Semantics/MarkupReferenceResolverTests.swift

Co-authored-by: David Rönnqvist <david.ronnqvist@gmail.com>

---------

Co-authored-by: David Rönnqvist <david.ronnqvist@gmail.com>
Kyle-Ye added a commit that referenced this pull request Oct 25, 2023
* Fix duplicated diagnostic issue

If the range’s source is not matched with the resolver’s source, we should not emit a problem

* Add test case for duplicated diagnostic issue

* Update Sources/SwiftDocC/Semantics/MarkupReferenceResolver.swift



* Update Tests/SwiftDocCTests/Semantics/MarkupReferenceResolverTests.swift



* Update Tests/SwiftDocCTests/Semantics/MarkupReferenceResolverTests.swift



---------

Co-authored-by: David Rönnqvist <david.ronnqvist@gmail.com>
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.

Duplicated diagnostic causing crash on DefaultDiagnosticConsoleFormatter convert and preview commands crash on certain Markdown pages

3 participants