Skip to content
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

Fix duplicated diagnostic issue #733

Merged
merged 5 commits into from
Oct 25, 2023
Merged

Conversation

Kyle-Ye
Copy link
Collaborator

@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
Copy link
Collaborator Author

Kyle-Ye commented Oct 15, 2023

@swift-ci please test

@Kyle-Ye
Copy link
Collaborator 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
Collaborator Author

Kyle-Ye commented Oct 17, 2023

@swift-ci please test

@Kyle-Ye
Copy link
Collaborator Author

Kyle-Ye commented Oct 23, 2023

Ping @binamaniar

@Kyle-Ye
Copy link
Collaborator 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
Collaborator 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 linked an issue Oct 25, 2023 that may be closed by this pull request
2 tasks
@Kyle-Ye
Copy link
Collaborator Author

Kyle-Ye commented Oct 25, 2023

@swift-ci please test

@Kyle-Ye Kyle-Ye merged commit 4e9d7f8 into apple:main Oct 25, 2023
2 checks passed
@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
3 participants