Require import of global symbols#898
Draft
MidnightDesign wants to merge 2 commits intocarthage-software:mainfrom
Draft
Require import of global symbols#898MidnightDesign wants to merge 2 commits intocarthage-software:mainfrom
MidnightDesign wants to merge 2 commits intocarthage-software:mainfrom
Conversation
Contributor
|
This should either be a linter or analyzer rule. Not a formatter setting. The linter can already detect these cases, it would just need a --fix option and possibly mark as unsafe. However I think both the formatter and linter don't have access to the codebase. So they don't know that "strlen" or any other symbol is in the global namespace. |
Contributor
Author
|
(Yeah, this probably should have been an issue instead of a PR. I just feel like a PR with failing test cases shows a bit more effort. : ) Whatever part of the toolchain this fit is definitely up for debate. My perspective was just that this is a missing feature compared to PHP CS Fixer, which is just a formatter. Anyway, this is a legitimate use case, right? |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
📌 What Does This PR Do?
Adds a formatter rule that requires global symbols like constants and functions to be imported.
🔍 Context & Motivation
Using fully qualified symbol names is a small performance and maintainability win because PHP doesn't have to check the current namespace for it. Importing symbols vs.
\strlen()just looks nicer IMO.📂 Affected Areas
It's arguable whether this is a formatter or a linter feature. Maybe the import of already-fully-qualified symbols is a formatter thing and the no-lookup part is a linter thing. To be discussed.
📝 Notes for Reviewers
I haven't implemented anything yet. Just added the tests as a basis for discussion. (I also don't know if I even could implement it given my limited Rust experience.)