Check for redundant internal and fileprivate

Sources/Frontend/Commands/ScanCommand.swift

@@ -51,8 +51,14 @@ struct ScanCommand: FrontendCommand {
     @Flag(help: "Retain all public declarations, recommended for framework/library projects")
     var retainPublic: Bool = defaultConfiguration.$retainPublic.defaultValue
 
-    @Flag(help: "Disable identification of redundant public accessibility")
-    var disableRedundantPublicAnalysis: Bool = defaultConfiguration.$disableRedundantPublicAnalysis.defaultValue
+	@Flag(help: "Disable identification of redundant public accessibility")
+	var disableRedundantPublicAnalysis: Bool = defaultConfiguration.$disableRedundantPublicAnalysis.defaultValue
+
+	@Flag(help: "Disable identification of redundant internal accessibility")
+	var disableRedundantInternalAnalysis: Bool = defaultConfiguration.$disableRedundantInternalAnalysis.defaultValue
+
+	@Flag(help: "Disable identification of redundant fileprivate accessibility")
+	var disableRedundantFilePrivateAnalysis: Bool = defaultConfiguration.$disableRedundantFilePrivateAnalysis.defaultValue
 
     @Flag(help: "Retain properties that are assigned, but never used")
     var retainAssignOnlyProperties: Bool = defaultConfiguration.$retainAssignOnlyProperties.defaultValue
@@ -124,6 +130,8 @@ struct ScanCommand: FrontendCommand {
         configuration.apply(\.$retainUnusedProtocolFuncParams, retainUnusedProtocolFuncParams)
         configuration.apply(\.$retainSwiftUIPreviews, retainSwiftUIPreviews)
         configuration.apply(\.$disableRedundantPublicAnalysis, disableRedundantPublicAnalysis)
+        configuration.apply(\.$disableRedundantInternalAnalysis, disableRedundantInternalAnalysis)
+        configuration.apply(\.$disableRedundantFilePrivateAnalysis, disableRedundantFilePrivateAnalysis)
         configuration.apply(\.$externalEncodableProtocols, externalEncodableProtocols)
         configuration.apply(\.$externalTestCaseClasses, externalTestCaseClasses)
         configuration.apply(\.$verbose, verbose)

Sources/Frontend/Formatters/OutputFormatter.swift

@@ -20,6 +20,10 @@ extension OutputFormatter {
             return "redundantProtocol"
         case .redundantPublicAccessibility:
             return "redundantPublicAccessibility"
+        case .redundantInternalAccessibility:
+            return "redundantInternalAccessibility"
+        case .redundantFilePrivateAccessibility:
+            return "redundantFilePrivateAccessibility"
         }
     }
 
@@ -49,6 +53,10 @@ extension OutputFormatter {
             case let .redundantPublicAccessibility(modules):
                 let modulesJoined = modules.sorted().joined(separator: ", ")
                 description += " is declared public, but not used outside of \(modulesJoined)"
+            case .redundantInternalAccessibility:
+				description += " is internal, but not used outside of file"
+            case .redundantFilePrivateAccessibility:
+				description += " is declared fileprivate, but not used outside of its scope"
             }
         } else {
             description += "unused"

Sources/PeripheryKit/ScanResult.swift

@@ -6,6 +6,8 @@ public struct ScanResult {
         case assignOnlyProperty
         case redundantProtocol(references: Set<Reference>)
         case redundantPublicAccessibility(modules: Set<String>)
+        case redundantInternalAccessibility(file: SourceFile)
+        case redundantFilePrivateAccessibility(file: SourceFile)
     }
 
     public let declaration: Declaration

Sources/PeripheryKit/ScanResultBuilder.swift

@@ -7,6 +7,8 @@ public struct ScanResultBuilder {
         let removableDeclarations = graph.unusedDeclarations.subtracting(assignOnlyProperties)
         let redundantProtocols = graph.redundantProtocols.filter { !removableDeclarations.contains($0.0) }
         let redundantPublicAccessibility = graph.redundantPublicAccessibility.filter { !removableDeclarations.contains($0.0) }
+        let redundantInternalAccessibility = graph.redundantInternalAccessibility.filter { !removableDeclarations.contains($0.0) }
+        let redundantFilePrivateAccessibility = graph.redundantFilePrivateAccessibility.filter { !removableDeclarations.contains($0.0) }
 
         let annotatedRemovableDeclarations: [ScanResult] = removableDeclarations.map {
             .init(declaration: $0, annotation: .unused)
@@ -20,10 +22,18 @@ public struct ScanResultBuilder {
         let annotatedRedundantPublicAccessibility: [ScanResult] = redundantPublicAccessibility.map {
             .init(declaration: $0.0, annotation: .redundantPublicAccessibility(modules: $0.1))
         }
+        let annotatedRedundantInternalAccessibility: [ScanResult] = redundantInternalAccessibility.map {
+            .init(declaration: $0.0, annotation: .redundantInternalAccessibility(file: $0.1))
+        }
+        let annotatedRedundantFilePrivateAccessibility: [ScanResult] = redundantFilePrivateAccessibility.map {
+            .init(declaration: $0.0, annotation: .redundantFilePrivateAccessibility(file: $0.1))
+        }
         let allAnnotatedDeclarations = annotatedRemovableDeclarations +
             annotatedAssignOnlyProperties +
             annotatedRedundantProtocols +
-            annotatedRedundantPublicAccessibility
+            annotatedRedundantPublicAccessibility +
+			annotatedRedundantInternalAccessibility +
+			annotatedRedundantFilePrivateAccessibility
 
         let result = allAnnotatedDeclarations
             .filter {

Sources/PeripheryKit/SourceGraph/Mutators/RedundantFilePrivateAccessibilityMarker.swift

@@ -0,0 +1,89 @@
+import Shared
+
+final class RedundantFilePrivateAccessibilityMarker: SourceGraphMutator {
+	private let graph: SourceGraph
+	private let configuration: Configuration
+
+	required init(graph: SourceGraph, configuration: Configuration) {
+		self.graph = graph
+		self.configuration = configuration
+	}
+
+	func mutate() throws {
+		guard !configuration.disableRedundantFilePrivateAnalysis else { return }
+
+		let nonExtensionKinds = graph.rootDeclarations.filter { !$0.kind.isExtensionKind }
+		let extensionKinds = graph.rootDeclarations.filter { $0.kind.isExtensionKind }
+
+		for decl in nonExtensionKinds {
+			try validate(decl)
+		}
+
+		for decl in extensionKinds {
+			try validateExtension(decl)
+		}
+	}
+
+	// MARK: - Private
+
+	private func validate(_ decl: Declaration) throws {
+		// Check if the declaration is explicitly fileprivate, and is referenced in multiple files.
+		if decl.accessibility.isExplicitly(.fileprivate) {
+			if try !isReferencedCrossScope(decl) {
+				// fileprivate accessibility is too exposed.
+				mark(decl)
+			}
+		} else {
+			// Look for descendent declarations that are not referenced outside of their scope that can be private
+			try markFilePrivateDescendentDeclarations(from: decl)
+		}
+	}
+
+	private func validateExtension(_ decl: Declaration) throws {
+		// Don't validate accessibility of extension, but validate the descendents.
+		try markFilePrivateDescendentDeclarations(from: decl)
+	}
+
+	private func mark(_ decl: Declaration) {
+		// This declaration may already be retained by a comment command.
+		guard !graph.isRetained(decl) else { return }
+		graph.markRedundantFilePrivateAccessibility(decl, file: decl.location.file)
+	}
+
+	private func markFilePrivateDescendentDeclarations(from decl: Declaration) throws {
+		for descDecl in descendentFilePrivateDeclarations(from: decl) {
+			if try !isReferencedCrossScope(descDecl) {
+				mark(descDecl)
+			}
+		}
+	}
+
+	private func isReferencedCrossScope(_ decl: Declaration) throws -> Bool {
+		let referenceDeclarations: Set<Declaration> = try nonTestableDeclarationsReferencing(decl)
+		let ancestralDeclarations: Set<Declaration> = decl.ancestralDeclarations
+		let otherDeclarations: Set<Declaration> = referenceDeclarations.subtracting(ancestralDeclarations)
+		// other declarations using this, but also this has SOME ancestral declaration to be considered referenced
+		// in a way that would require fileprivate not private
+		return !otherDeclarations.isEmpty && !ancestralDeclarations.isEmpty
+	}
+
+	/*
+	 (lldb) po graph.references(to: decl).map { $0.parent?.ancestralDeclarations }
+
+
+	 (lldb) po graph.references(to: decl).map { Array($0.parent?.ancestralDeclarations ?? []) }.flatMap { $0 }
+
+
+
+	 */
+
+	private func nonTestableDeclarationsReferencing(_ decl: Declaration) throws -> Set<Declaration> {
+		let result = Set(graph.references(to: decl).map { Array($0.parent?.ancestralDeclarations ?? []) }.flatMap { $0 })
+		return result
+	}
+
+	private func descendentFilePrivateDeclarations(from decl: Declaration) -> Set<Declaration> {
+		let fileprivateDeclarations = decl.declarations.filter { !$0.isImplicit && $0.accessibility.isExplicitly(.fileprivate) }
+		return Set(fileprivateDeclarations.flatMap { descendentFilePrivateDeclarations(from: $0) }).union(fileprivateDeclarations)
+	}
+}

Sources/PeripheryKit/SourceGraph/Mutators/RedundantInternalAccessibilityMarker.swift

@@ -0,0 +1,86 @@
+import Shared
+
+final class RedundantInternalAccessibilityMarker: SourceGraphMutator {
+    private let graph: SourceGraph
+    private let configuration: Configuration
+
+    required init(graph: SourceGraph, configuration: Configuration) {
+        self.graph = graph
+        self.configuration = configuration
+    }
+
+    func mutate() throws {
+        guard !configuration.disableRedundantInternalAnalysis else { return }
+
+		let nonExtensionKinds = graph.rootDeclarations.filter { !$0.kind.isExtensionKind }
+		let extensionKinds = graph.rootDeclarations.filter { $0.kind.isExtensionKind }
+
+		for decl in nonExtensionKinds {
+			try validate(decl)
+		}
+
+		for decl in extensionKinds {
+			try validateExtension(decl)
+		}
+    }
+
+    // MARK: - Private
+
+	private func validate(_ decl: Declaration) throws {
+        // Check if the declaration is [explicitly/implicitly] internal, and is referenced in multiple files.
+        if decl.accessibility.value == .internal {
+            if try !isReferencedCrossFile(decl) {
+                mark(decl)
+            }
+
+        } else {
+            try markInternalDescendentDeclarations(from: decl)
+        }
+    }
+
+	private func validateExtension(_ decl: Declaration) throws {
+		// Don't validate accessibility of extension, but validate the descendents.
+		try markInternalDescendentDeclarations(from: decl)
+	}
+
+    private func mark(_ decl: Declaration) {
+        // This declaration may already be retained by a comment command.
+        guard !graph.isRetained(decl) else { return }
+        graph.markRedundantInternalAccessibility(decl, file: decl.location.file)
+    }
+
+    private func markInternalDescendentDeclarations(from decl: Declaration) throws {
+        for descDecl in descendentInternalDeclarations(from: decl) {
+			if try !isReferencedCrossFile(descDecl) && !isImplementingProtocol(descDecl) && !descDecl.isOverride {
+				mark(descDecl)
+			}
+        }
+    }
+
+	private func isImplementingProtocol(_ decl: Declaration) throws -> Bool {
+		guard let parent: Declaration = decl.parent else { return false }
+		let protocols: [Reference] = parent.related.filter { $0.kind == .protocol }
+		guard !protocols.isEmpty else { return false }		// no protocol at all
+		let declarations: [Declaration] = protocols.compactMap { graph.explicitDeclaration(withUsr: $0.usr) }
+		guard !declarations.isEmpty else { return true }	// If protocol isn't actually defined here, we don't have a way to match, so assume it's OK
+
+		let protocolMethods: [Declaration] = declarations.map { $0.declarations }.flatMap { $0 }
+		let matchingProtocol: Declaration? = protocolMethods.first { $0.kind == decl.kind && $0.name == decl.name }
+		return matchingProtocol != nil
+	}
+
+    private func isReferencedCrossFile(_ decl: Declaration) throws -> Bool {
+        let referenceFiles = try nonTestableFilesReferencing(decl)
+        return !referenceFiles.subtracting([decl.location.file]).isEmpty
+    }
+
+    private func nonTestableFilesReferencing(_ decl: Declaration) throws -> Set<SourceFile> {
+        let referenceFiles = Set(graph.references(to: decl).map { $0.location.file })
+        return referenceFiles
+    }
+
+    private func descendentInternalDeclarations(from decl: Declaration) -> Set<Declaration> {
+        let internalDeclarations = decl.declarations.filter { $0.accessibility.value == .internal }
+        return Set(internalDeclarations.flatMap { descendentInternalDeclarations(from: $0) }).union(internalDeclarations)
+    }
+}

Sources/PeripheryKit/SourceGraph/SourceGraph.swift

@@ -10,6 +10,8 @@ public final class SourceGraph {
     private(set) public var redundantProtocols: [Declaration: Set<Reference>] = [:]
     private(set) public var rootDeclarations: Set<Declaration> = []
     private(set) public var redundantPublicAccessibility: [Declaration: Set<String>] = [:]
+    private(set) public var redundantInternalAccessibility: [Declaration: SourceFile] = [:]
+    private(set) public var redundantFilePrivateAccessibility: [Declaration: SourceFile] = [:]
 
     private(set) var rootReferences: Set<Reference> = []
     private(set) var allReferences: Set<Reference> = []
@@ -80,6 +82,30 @@ public final class SourceGraph {
         }
     }
 
+    func markRedundantInternalAccessibility(_ declaration: Declaration, file: SourceFile) {
+        withLock {
+            redundantInternalAccessibility[declaration] = file
+        }
+    }
+
+    func unmarkRedundantInternalAccessibility(_ declaration: Declaration) {
+        withLock {
+            _ = redundantInternalAccessibility.removeValue(forKey: declaration)
+        }
+    }
+
+    func markRedundantFilePrivateAccessibility(_ declaration: Declaration, file: SourceFile) {
+        withLock {
+            redundantFilePrivateAccessibility[declaration] = file
+        }
+    }
+
+    func unmarkRedundantFilePrivateAccessibility(_ declaration: Declaration) {
+        withLock {
+            _ = redundantFilePrivateAccessibility.removeValue(forKey: declaration)
+        }
+    }
+
     func markIgnored(_ declaration: Declaration) {
         withLock {
             _ = ignoredDeclarations.insert(declaration)

Sources/PeripheryKit/SourceGraph/SourceGraphMutatorRunner.swift

@@ -11,7 +11,9 @@ public final class SourceGraphMutatorRunner {
         AccessibilityCascader.self,
         ObjCAccessibleRetainer.self,
         // Must come before ExtensionReferenceBuilder so that it can detect redundant accessibility on extensions.
-        RedundantExplicitPublicAccessibilityMarker.self,
+		RedundantExplicitPublicAccessibilityMarker.self,
+		RedundantInternalAccessibilityMarker.self,
+		RedundantFilePrivateAccessibilityMarker.self,
         GenericClassAndStructConstructorReferenceBuilder.self,
         // Must come before ProtocolExtensionReferenceBuilder because it removes references
         // from the extension to the protocol, thus making them appear to be unknown.

Sources/Shared/Configuration.swift

@@ -71,6 +71,12 @@ public final class Configuration {
     @Setting(key: "disable_redundant_public_analysis", defaultValue: false)
     public var disableRedundantPublicAnalysis: Bool
 
+    @Setting(key: "disable_redundant_static_analysis", defaultValue: false)
+    public var disableRedundantInternalAnalysis: Bool
+
+    @Setting(key: "disable_redundant_fileprivate_analysis", defaultValue: false)
+    public var disableRedundantFilePrivateAnalysis: Bool
+
     @Setting(key: "verbose", defaultValue: false)
     public var verbose: Bool
 
@@ -177,6 +183,14 @@ public final class Configuration {
             config[$disableRedundantPublicAnalysis.key] = disableRedundantPublicAnalysis
         }
 
+        if $disableRedundantInternalAnalysis.hasNonDefaultValue {
+            config[$disableRedundantInternalAnalysis.key] = disableRedundantInternalAnalysis
+        }
+
+        if $disableRedundantFilePrivateAnalysis.hasNonDefaultValue {
+            config[$disableRedundantFilePrivateAnalysis.key] = disableRedundantFilePrivateAnalysis
+        }
+
         if $verbose.hasNonDefaultValue {
             config[$verbose.key] = verbose
         }
@@ -263,6 +277,10 @@ public final class Configuration {
                 $retainSwiftUIPreviews.assign(value)
             case $disableRedundantPublicAnalysis.key:
                 $disableRedundantPublicAnalysis.assign(value)
+            case $disableRedundantInternalAnalysis.key:
+                $disableRedundantInternalAnalysis.assign(value)
+            case $disableRedundantFilePrivateAnalysis.key:
+                $disableRedundantFilePrivateAnalysis.assign(value)
             case $verbose.key:
                 $verbose.assign(value)
             case $quiet.key:
@@ -303,6 +321,8 @@ public final class Configuration {
         $retainUnusedProtocolFuncParams.reset()
         $retainSwiftUIPreviews.reset()
         $disableRedundantPublicAnalysis.reset()
+        $disableRedundantInternalAnalysis.reset()
+        $disableRedundantFilePrivateAnalysis.reset()
         $externalEncodableProtocols.reset()
         $externalTestCaseClasses.reset()
         $verbose.reset()

Tests/AccessibilityTests/AccessibilityProject/Sources/MainTarget/main.swift

@@ -57,3 +57,10 @@ _ = InternalProtocolRefiningPublicProtocolRetainer()
 // Closure
 let _ = PublicTypeUsedInPublicClosureRetainer().closure
 
+// Support for tests for items being overly public
+
+_ = RedundantInternalClassComponents()
+_ = NotRedundantInternalClassComponents()
+_ = NotRedundantInternalClassComponents_Support()
+_ = RedundantFilePrivateComponents()
+_ = NotRedundantFilePrivateComponents()