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 Package.swift diagnostics line off by 1 for tools versions <= 5.7 #7795

Merged
merged 4 commits into from
Jul 19, 2024

Conversation

bripeticca
Copy link
Contributor

Fixes issue #7688

SwiftPM was implicitly prepending the manifest with 'import Foundation' for toolchains using Swift toolchain versions 5.7 and under. The original file itself was never overwritten, thus the diagnostics being reported would always present on the next line.

This is a quick fix that appends the same line to the end of the manifest; doing so assures that the intended behaviour doesn't change, but allows for correct diagnostics to be reported.

Motivation:

Diagnostics would present on the incorrect line for Package.swift files using tools version 5.7 and under.

Modifications:

When evaluating the manifest, a temporary directory copies the contents of the manifest to a temporary file; rather than prepending the import to Foundation at the top of the file (causing all lines in the diagnostic reports to be off by one), append the import to the bottom of the temporary manifest when dealing with tools versions 5.7 and under.

Result:

The intended behaviour that requires users to explicitly import Foundation doesn't change for Swift versions 5.8 and up; diagnostics reported for Package.swift files for those using swift <= 5.7 will now show on the correct line.

SPM was implicitly preprending the manifest with 'import Foundation' for
toolchains using swift version 5.7 and under. The original file itself
was never overwritten, thus the diagnostics being reported would always
present on the next line.

This is a quick fix that appends the line to the end of the file;
doing so assures that the intended behaviour doesn't change, but allows
for correct diagnostics to be reported.
@bripeticca bripeticca marked this pull request as ready for review July 18, 2024 19:03
@bripeticca
Copy link
Contributor Author

@swift-ci please test

Previous testing worked off of the incorrect line number that was
reported by diagnostics.
@bripeticca
Copy link
Contributor Author

@swift-ci please test

@MaxDesiatov
Copy link
Contributor

@swift-ci test windows

Copy link
Contributor

@MaxDesiatov MaxDesiatov left a comment

Choose a reason for hiding this comment

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

Great find, thanks!

@MaxDesiatov MaxDesiatov enabled auto-merge (squash) July 19, 2024 08:12
@MaxDesiatov MaxDesiatov added package manifests changes to package manifest APIs diagnostics labels Jul 19, 2024
@MaxDesiatov MaxDesiatov merged commit 1990046 into swiftlang:main Jul 19, 2024
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
diagnostics package manifests changes to package manifest APIs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants