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

Check for redundant internal and fileprivate #605

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

danwood
Copy link

@danwood danwood commented Apr 20, 2023

As somebody brought up a while ago in this discussion, I've long felt it would be great to be able to highlight code that is more public than it ought to be. For instance, static (implicit or explicit) that really ought to be fileprivate or private; or fileprivate that ought to be private. (Note that periphery already checks for redundantly public declarations.)

Since @ileitch agreed that it would be a good feature, but hasn't had a change to get around to it in almost 2 years — my how time flies — I thought I would take a crack at it.

I think I have it working pretty well. There may be some corner cases I haven't considered; especially considering that there is some code in RedundantExplicitPublicAccessibilityMarker.swift (upon which much of my code is based) that I couldn't quite figure out if it needed to be brought over to the other tests.

One limitation that I haven't figured out a workaround for is when an implementation of a protocol needs to be have comparable visibility and thus shouldn't generate a warning if it's internal scope. The code can confirm if it's a protocol defined in the code, but if it's an API protocol (such as conforming to AppKit or UIKit) then we can't get to the Declaration given a Reference. Maybe there is a technique that I couldn't figure out. For now, I figured it was safer to not treat this as a problem if we can't find the reference, rather than generate an annoying false positive.

Hopefully this is close to mergeable. There are probably some conventions that you'd like to stick to that I wasn't aware of, so I'll be happy to tweak it a bit … Or maybe it's just the foundation upon which further improvements can be built.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant