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

Add rules to discourage #file and #filePath #231

Merged
merged 9 commits into from
Jun 15, 2023
Merged

Conversation

jqsilver
Copy link
Contributor

@jqsilver jqsilver commented Jun 15, 2023

Summary

Add lint rules to discourage the use of #file and #filePath literals (macros in 5.9).

In addition, my editor removed some trailing whitespace - let me know if I should revert that.

Reasoning

  1. #file's behavior is changing in future Swift versions (https://developer.apple.com/documentation/swift/file), from the problematic behavior of #filePath to the more ideal behavior of #fileID. We should just use the literal/macro with explicit and unchanging behavior.
  2. The documentation for #filePath says "Because #fileID doesn’t embed the full path to the source file, unlike #filePath, it gives you better privacy and reduces the size of the compiled binary. Avoid using #filePath outside of tests, build scripts, or other code that doesn’t become part of the shipping program." https://developer.apple.com/documentation/swift/filepath, so we should reflect this guidance in our lint rules.

Please react with 👍/👎 if you agree or disagree with this proposal.

@jqsilver jqsilver requested a review from calda June 15, 2023 15:30
@jqsilver jqsilver requested a review from swiftal64 June 15, 2023 15:32
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
@calda
Copy link
Member

calda commented Jun 15, 2023

README.md Outdated Show resolved Hide resolved
jqsilver and others added 6 commits June 15, 2023 08:55
Co-authored-by: Cal Stephens <cal@calstephens.tech>
Co-authored-by: Cal Stephens <cal@calstephens.tech>
Co-authored-by: Cal Stephens <cal@calstephens.tech>
@jqsilver
Copy link
Contributor Author

@calda I added a lint rule for #file. Should I also add one for #filePath? It's harder to exempt tests and tools from a rule about #filePath

@calda
Copy link
Member

calda commented Jun 15, 2023

We can follow how the no_direct_standard_out_logs rule is set up, and not exempt test targets in the swiftlint.yml file in this repo (even though we exempt test targets / demo modules etc in the swiftlint.yml file in the apps repo). Folks using our style tool can manually add // swiftlint:disable annotations as necessary (this is what we do in the Epoxy and Lottie repos).

@jqsilver jqsilver merged commit 44a4f76 into master Jun 15, 2023
3 checks passed
@jqsilver jqsilver deleted the ab-file-macros branch June 15, 2023 16:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants