Add redundant internal and fileprivate analysis to go alongside redundant public analysis.

[danwood]

It's useful to make your swift types be as private as possible, to encourage encapsulation. This improvement adds new checks for types that are fileprivate or (implicitly/explicitly) internal, and lets you know if they could be made more private then they are currently.

(This is a reworking of a branch and pull request I did a year or so ago. I'm trying again.)

[ileitch]

Thanks for reworking this. Let's give it a shot, and see if people find it useful.

[danwood]

Just noticed that this got looked at. I'll clean stuff up so we can try again!

[danwood]

ok @ileitch I dealt with your comments; also made some big improvements to the tests which I realized I hadn't quite finished up yet. I think we're good to go now.

[ileitch]

@danwood Can you please take a look at the failures? They indicate that there aren't any tests using assertRedundantInternalAccessibility.

[danwood]

@ileitch I am working on updating my pull request to address your issue of missing tests. Yes, I see now that the assertions like "assertRedundantInternalAccessibility" were not actually invoked, so I need to build some new testing code. The problem is that the warnings you indicated have disappeared from this pull request, and I don't know how to regenerate those warnings so I can confirm that I've implemented everything I need to implement. All the tests pass when I use swift test so I'm missing something here. Thanks.

[ileitch]

@ileitch I am working on updating my pull request to address your issue of missing tests. Yes, I see now that the assertions like "assertRedundantInternalAccessibility" were not actually invoked, so I need to build some new testing code. The problem is that the warnings you indicated have disappeared from this pull request, and I don't know how to regenerate those warnings so I can confirm that I've implemented everything I need to implement. All the tests pass when I use swift test so I'm missing something here. Thanks.

It was the scan check that was failing. You can invoke it locally with mise r scan.

[danwood]

@ileitch I will try to dig into these errors, which have appeared again. Not sure if I can address the linux errors, though - I don't have a linux installation.

I'm not famiilar with mise r scan — a google search yielded this https://mise.jdx.dev; is this what you are talking about? I see some file names containing "mise" in the repository but I don't see any documentation on how it's related to periphery. Is there some documentation I'm missing?

[danwood]

@ileitch from the output here, it looks like all but one of the "mise" complaints were about the same one item in my codebase that I have since fixed; the remaining one was about the formatting of the Sources/BUILD.bazel file, which just had a couple of additions for my files, but I can't really figure out what it is complaining about. I'm not using Bazel, just swift build to build, so maybe you have a better understanding of what is wrong with the file?

[ileitch]

mise r lint and mise r gen-bazel-rules should resolve all of the lint issues.

There's a real issue with the Glob.path property, however. The original fileprivate accessibility is correct, because it is referenced on line 18 outside of the Glob class.

[danwood]

OK, I took a guess that mise was available on homebrew, and it seems to have worked. Hopefully they warnings all go away.

[ileitch]

The issue with the 'path' property is that 'fileprivate' was incorrectly identified as redundant, but it is actually the most appropriate accessibility level. I don't think simply removing the accessibility is the right thing to do, because it represents a real bug in your PR.

[danwood]

OK I think we are good to go.

Note: I see "1 requested change" here on github but no indication of what the actual request is. Maybe it's something I already resolved, like fixing the access back to fileprivate?

BTW, I am also working on an enhancement that will flag redundant access declarations; e.g. it would point out redundant private declarations found in FilePath+Glob.swift, TypeSyntaxInspectorTest.swift, and SwiftIndexer.swift. (That's where you have a private class or struct, with private properties and functions declared inside). But that's probably best to put that in a separate PR once we figure this one out.

[ileitch]

@danwood Unfortunatley, I had to revert this change in #1038, as I discovered some blocker issues with the internal accessibility analysis. See PR for details.