OpenPeeDeeP / depguard

Go linter that checks if package imports are in a list of acceptable packages.
GNU General Public License v3.0
138 stars 15 forks source link

Compile allow and deny list as globs to allow wildcards #67

Open philwitty opened 11 months ago

philwitty commented 11 months ago
philwitty commented 10 months ago

I am contemplating if this matters. But there was a performance benefit to only allowing prefixing instead of globs. Globs require to go through the array from the start to the end (the sorting that was done is no longer needed and maybe not expected).

Doing string comparison in comparison is cheaper and I can do a binary search (sort.Search) requiring potentially even fewer comparisons against what could be pretty lengthy slice (think mono repository with many packages).

There are significantly fewer files than imports so was willing to take the hit of globs there. Plus, globing against a well structured package (one controlled by the maintainers of the linter configuration) makes logical sense. But not every go project is built the same. So globing at package level seemed a bit like false security (don't include any package with the name test?). Prefixing made the most sense as I may not trust X organization and I can just exclude it with a prefix...

Really good points, we certainly could optimise the globbing if performance is an issue (i.e. using a custom globber which given the well defined package names that sounds reasonable), as there isn't any cleverness in the current matching this shouldn't be a noticeable performance decrease I think.

Not sure I understand your second point about the use, I should have made my use case clear in the PR, we wanted to have a shared linter config across many repos but ensure none of them were using any our-organisation/{repo}/test/ packages in non test code