From 98c4d03dd30eb1c59e9467dfdb52c1f71d3c3fe8 Mon Sep 17 00:00:00 2001 From: Dan Wood Date: Tue, 18 Apr 2023 14:30:05 -0700 Subject: [PATCH] Check for redundant internal and fileprivate --- Sources/Frontend/Commands/ScanCommand.swift | 12 ++- .../Frontend/Formatters/OutputFormatter.swift | 8 ++ Sources/PeripheryKit/ScanResult.swift | 2 + Sources/PeripheryKit/ScanResultBuilder.swift | 12 ++- ...undantFilePrivateAccessibilityMarker.swift | 89 +++++++++++++++++++ ...RedundantInternalAccessibilityMarker.swift | 86 ++++++++++++++++++ .../SourceGraph/SourceGraph.swift | 26 ++++++ .../SourceGraphMutatorRunner.swift | 4 +- Sources/Shared/Configuration.swift | 20 +++++ .../Sources/MainTarget/main.swift | 7 ++ .../NotRedundantFilePrivateComponents.swift | 65 ++++++++++++++ .../NotRedundantInternalClassComponents.swift | 61 +++++++++++++ ...ndantInternalClassComponents_Support.swift | 27 ++++++ .../RedundantFilePrivateComponents.swift | 47 ++++++++++ .../TargetA/RedundantInternalComponents.swift | 50 +++++++++++ ...edundantFilePrivateAccessibilityTest.swift | 68 ++++++++++++++ .../RedundantInternalAccessibilityTest.swift | 75 ++++++++++++++++ Tests/Shared/SourceGraphTestCase.swift | 49 ++++++++++ .../UIKitProject/AppDelegate.swift | 6 ++ 19 files changed, 710 insertions(+), 4 deletions(-) create mode 100644 Sources/PeripheryKit/SourceGraph/Mutators/RedundantFilePrivateAccessibilityMarker.swift create mode 100644 Sources/PeripheryKit/SourceGraph/Mutators/RedundantInternalAccessibilityMarker.swift create mode 100644 Tests/AccessibilityTests/AccessibilityProject/Sources/TargetA/NotRedundantFilePrivateComponents.swift create mode 100644 Tests/AccessibilityTests/AccessibilityProject/Sources/TargetA/NotRedundantInternalClassComponents.swift create mode 100644 Tests/AccessibilityTests/AccessibilityProject/Sources/TargetA/NotRedundantInternalClassComponents_Support.swift create mode 100644 Tests/AccessibilityTests/AccessibilityProject/Sources/TargetA/RedundantFilePrivateComponents.swift create mode 100644 Tests/AccessibilityTests/AccessibilityProject/Sources/TargetA/RedundantInternalComponents.swift create mode 100644 Tests/AccessibilityTests/RedundantFilePrivateAccessibilityTest.swift create mode 100644 Tests/AccessibilityTests/RedundantInternalAccessibilityTest.swift diff --git a/Sources/Frontend/Commands/ScanCommand.swift b/Sources/Frontend/Commands/ScanCommand.swift index c5c609e45..6a317de20 100644 --- a/Sources/Frontend/Commands/ScanCommand.swift +++ b/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 @@ -113,6 +119,8 @@ struct ScanCommand: FrontendCommand { configuration.apply(\.$retainObjcAccessible, retainObjcAccessible) configuration.apply(\.$retainUnusedProtocolFuncParams, retainUnusedProtocolFuncParams) configuration.apply(\.$disableRedundantPublicAnalysis, disableRedundantPublicAnalysis) + configuration.apply(\.$disableRedundantInternalAnalysis, disableRedundantInternalAnalysis) + configuration.apply(\.$disableRedundantFilePrivateAnalysis, disableRedundantFilePrivateAnalysis) configuration.apply(\.$externalEncodableProtocols, externalEncodableProtocols) configuration.apply(\.$verbose, verbose) configuration.apply(\.$quiet, quiet) diff --git a/Sources/Frontend/Formatters/OutputFormatter.swift b/Sources/Frontend/Formatters/OutputFormatter.swift index 827b94b7e..acd2e0bea 100644 --- a/Sources/Frontend/Formatters/OutputFormatter.swift +++ b/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.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" diff --git a/Sources/PeripheryKit/ScanResult.swift b/Sources/PeripheryKit/ScanResult.swift index 50a9145e5..d0bc06ad7 100644 --- a/Sources/PeripheryKit/ScanResult.swift +++ b/Sources/PeripheryKit/ScanResult.swift @@ -6,6 +6,8 @@ public struct ScanResult { case assignOnlyProperty case redundantProtocol(references: Set) case redundantPublicAccessibility(modules: Set) + case redundantInternalAccessibility(file: SourceFile) + case redundantFilePrivateAccessibility(file: SourceFile) } public let declaration: Declaration diff --git a/Sources/PeripheryKit/ScanResultBuilder.swift b/Sources/PeripheryKit/ScanResultBuilder.swift index ef0f56416..e44106254 100644 --- a/Sources/PeripheryKit/ScanResultBuilder.swift +++ b/Sources/PeripheryKit/ScanResultBuilder.swift @@ -10,6 +10,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) @@ -23,10 +25,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 { diff --git a/Sources/PeripheryKit/SourceGraph/Mutators/RedundantFilePrivateAccessibilityMarker.swift b/Sources/PeripheryKit/SourceGraph/Mutators/RedundantFilePrivateAccessibilityMarker.swift new file mode 100644 index 000000000..e8738ad06 --- /dev/null +++ b/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 = try nonTestableDeclarationsReferencing(decl) + let ancestralDeclarations: Set = decl.ancestralDeclarations + let otherDeclarations: Set = 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 { + let result = Set(graph.references(to: decl).map { Array($0.parent?.ancestralDeclarations ?? []) }.flatMap { $0 }) + return result + } + + private func descendentFilePrivateDeclarations(from decl: Declaration) -> Set { + let fileprivateDeclarations = decl.declarations.filter { !$0.isImplicit && $0.accessibility.isExplicitly(.fileprivate) } + return Set(fileprivateDeclarations.flatMap { descendentFilePrivateDeclarations(from: $0) }).union(fileprivateDeclarations) + } +} diff --git a/Sources/PeripheryKit/SourceGraph/Mutators/RedundantInternalAccessibilityMarker.swift b/Sources/PeripheryKit/SourceGraph/Mutators/RedundantInternalAccessibilityMarker.swift new file mode 100644 index 000000000..70b6fc4d1 --- /dev/null +++ b/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 { + let referenceFiles = Set(graph.references(to: decl).map { $0.location.file }) + return referenceFiles + } + + private func descendentInternalDeclarations(from decl: Declaration) -> Set { + let internalDeclarations = decl.declarations.filter { $0.accessibility.value == .internal } + return Set(internalDeclarations.flatMap { descendentInternalDeclarations(from: $0) }).union(internalDeclarations) + } +} diff --git a/Sources/PeripheryKit/SourceGraph/SourceGraph.swift b/Sources/PeripheryKit/SourceGraph/SourceGraph.swift index 16bd57d10..816bd6582 100644 --- a/Sources/PeripheryKit/SourceGraph/SourceGraph.swift +++ b/Sources/PeripheryKit/SourceGraph/SourceGraph.swift @@ -10,6 +10,8 @@ public final class SourceGraph { private(set) public var redundantProtocols: [Declaration: Set] = [:] private(set) public var rootDeclarations: Set = [] private(set) public var redundantPublicAccessibility: [Declaration: Set] = [:] + private(set) public var redundantInternalAccessibility: [Declaration: SourceFile] = [:] + private(set) public var redundantFilePrivateAccessibility: [Declaration: SourceFile] = [:] private(set) var rootReferences: Set = [] private(set) var allReferences: Set = [] @@ -81,6 +83,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) diff --git a/Sources/PeripheryKit/SourceGraph/SourceGraphMutatorRunner.swift b/Sources/PeripheryKit/SourceGraph/SourceGraphMutatorRunner.swift index dfc73ca39..41ee7feee 100644 --- a/Sources/PeripheryKit/SourceGraph/SourceGraphMutatorRunner.swift +++ b/Sources/PeripheryKit/SourceGraph/SourceGraphMutatorRunner.swift @@ -10,7 +10,9 @@ public final class SourceGraphMutatorRunner { // Must come before ExtensionReferenceBuilder. AccessibilityCascader.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. diff --git a/Sources/Shared/Configuration.swift b/Sources/Shared/Configuration.swift index a2c8b3cc6..d21814757 100644 --- a/Sources/Shared/Configuration.swift +++ b/Sources/Shared/Configuration.swift @@ -62,6 +62,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 @@ -156,6 +162,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 } @@ -236,6 +250,10 @@ public final class Configuration { $retainUnusedProtocolFuncParams.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: @@ -274,6 +292,8 @@ public final class Configuration { $retainObjcAccessible.reset() $retainUnusedProtocolFuncParams.reset() $disableRedundantPublicAnalysis.reset() + $disableRedundantInternalAnalysis.reset() + $disableRedundantFilePrivateAnalysis.reset() $externalEncodableProtocols.reset() $verbose.reset() $quiet.reset() diff --git a/Tests/AccessibilityTests/AccessibilityProject/Sources/MainTarget/main.swift b/Tests/AccessibilityTests/AccessibilityProject/Sources/MainTarget/main.swift index 2d44e240e..a86a79474 100644 --- a/Tests/AccessibilityTests/AccessibilityProject/Sources/MainTarget/main.swift +++ b/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() diff --git a/Tests/AccessibilityTests/AccessibilityProject/Sources/TargetA/NotRedundantFilePrivateComponents.swift b/Tests/AccessibilityTests/AccessibilityProject/Sources/TargetA/NotRedundantFilePrivateComponents.swift new file mode 100644 index 000000000..794a487ae --- /dev/null +++ b/Tests/AccessibilityTests/AccessibilityProject/Sources/TargetA/NotRedundantFilePrivateComponents.swift @@ -0,0 +1,65 @@ +// +// NotRedundantFilePrivateComponents.swift +// +// +// Created by Dan Wood on 4/18/23. +// + +import Foundation + +public class NotRedundantFilePrivateComponents { + + public init() { + let _ = SubclassCorrectlyFilePrivate() + let _ = StructCorrectlyFilePrivate() + let _ = EnumCorrectlyFilePrivate.a + Self.aFuncCorrectlyFilePrivate() + let _ = Self.aVarCorrectlyFilePrivate + } + + // fileprivate is correct here since they are used in the other class below. + fileprivate class SubclassCorrectlyFilePrivate {} + fileprivate struct StructCorrectlyFilePrivate {} + fileprivate enum EnumCorrectlyFilePrivate { + case a + } + fileprivate static func aFuncCorrectlyFilePrivate() {} + fileprivate static var aVarCorrectlyFilePrivate: Int = 0 +} + +private class AnotherClassUsingTheseComponents { + + public init() { + let _ = NotRedundantFilePrivateComponents.SubclassCorrectlyFilePrivate() + let _ = NotRedundantFilePrivateComponents.StructCorrectlyFilePrivate() + let _ = NotRedundantFilePrivateComponents.EnumCorrectlyFilePrivate.a + NotRedundantFilePrivateComponents.aFuncCorrectlyFilePrivate() + let _ = NotRedundantFilePrivateComponents.aVarCorrectlyFilePrivate + } + +} + + +extension NotRedundantFilePrivateComponents { + + // fileprivate is correct here since they are used in the other class below. + fileprivate class ExtensionSubclassCorrectlyFilePrivate {} + fileprivate struct ExtensionStructCorrectlyFilePrivate {} + fileprivate enum ExtensionEnumCorrectlyFilePrivate { + case a + } + fileprivate static func aExtensionFuncCorrectlyFilePrivate() {} + fileprivate static var aExtensionVarCorrectlyFilePrivate: Int = 0 +} + +extension AnotherClassUsingTheseComponents { + + private func something() { + let _ = NotRedundantFilePrivateComponents.ExtensionSubclassCorrectlyFilePrivate() + let _ = NotRedundantFilePrivateComponents.ExtensionStructCorrectlyFilePrivate() + let _ = NotRedundantFilePrivateComponents.ExtensionEnumCorrectlyFilePrivate.a + NotRedundantFilePrivateComponents.aExtensionFuncCorrectlyFilePrivate() + let _ = NotRedundantFilePrivateComponents.aExtensionVarCorrectlyFilePrivate + } + +} diff --git a/Tests/AccessibilityTests/AccessibilityProject/Sources/TargetA/NotRedundantInternalClassComponents.swift b/Tests/AccessibilityTests/AccessibilityProject/Sources/TargetA/NotRedundantInternalClassComponents.swift new file mode 100644 index 000000000..6a3c79425 --- /dev/null +++ b/Tests/AccessibilityTests/AccessibilityProject/Sources/TargetA/NotRedundantInternalClassComponents.swift @@ -0,0 +1,61 @@ +// +// NotRedundantInternalClassComponents.swift +// +// +// Created by Dan Wood on 4/18/23. +// + +import Cocoa + +public class NotRedundantInternalClassComponents { + + public init() { + let _ = NotRedundantInternalSubclass() + let _ = NotRedundantInternalStruct() + let _ = NotRedundantInternalEnum.a + Self.aNotRedundantInternalFunc() + let _ = Self.aNotRedundantInternalVar + } + + // Declared here, but also used in NotRedundantInternalClassComponents_Support so they shouldn't be private + class NotRedundantInternalSubclass {} + struct NotRedundantInternalStruct {} + enum NotRedundantInternalEnum { + case a + } + static func aNotRedundantInternalFunc() {} + static var aNotRedundantInternalVar: Int = 0 + +} + +extension NotRedundantInternalClassComponents { + + class ExtensionNotRedundantInternalSubclass {} + struct ExtensionNotRedundantInternalStruct {} + enum ExtensionNotRedundantInternalEnum { + case a + } + static func aExtensionNotRedundantInternalFunc() {} + static var aExtensionNotRedundantInternalVar: Int = 0 + + +} + + +public protocol MyProtocol { + + @available(iOS 2.0, *) + static func protocolMethod() +} + + +private class NotRedundantInternalClassCompontentsConformingToProtocol: MyProtocol { + + static func protocolMethod() { } +} + +private class NotRedundantInternalClassCompontentsConformingToAppKitProtocol: NSObject, NSApplicationDelegate { + + func applicationDidFinishLaunching(_ notification: Notification) {} + +} diff --git a/Tests/AccessibilityTests/AccessibilityProject/Sources/TargetA/NotRedundantInternalClassComponents_Support.swift b/Tests/AccessibilityTests/AccessibilityProject/Sources/TargetA/NotRedundantInternalClassComponents_Support.swift new file mode 100644 index 000000000..2b6d2f9ca --- /dev/null +++ b/Tests/AccessibilityTests/AccessibilityProject/Sources/TargetA/NotRedundantInternalClassComponents_Support.swift @@ -0,0 +1,27 @@ +// +// NotRedundantInternalClassComponents_Support.swift +// +// +// Created by Dan Wood on 4/18/23. +// + +import Foundation + +// A different file using the components of NotRedundantInternalClassComponents that shouldn't be private +public class NotRedundantInternalClassComponents_Support { + + public init() { + let _ = NotRedundantInternalClassComponents.NotRedundantInternalSubclass() + let _ = NotRedundantInternalClassComponents.NotRedundantInternalStruct() + let _ = NotRedundantInternalClassComponents.NotRedundantInternalEnum.a + NotRedundantInternalClassComponents.aNotRedundantInternalFunc() + let _ = NotRedundantInternalClassComponents.aNotRedundantInternalVar + + let _ = NotRedundantInternalClassComponents.ExtensionNotRedundantInternalSubclass() + let _ = NotRedundantInternalClassComponents.ExtensionNotRedundantInternalStruct() + let _ = NotRedundantInternalClassComponents.ExtensionNotRedundantInternalEnum.a + NotRedundantInternalClassComponents.aExtensionNotRedundantInternalFunc() + let _ = NotRedundantInternalClassComponents.aExtensionNotRedundantInternalVar + } + +} diff --git a/Tests/AccessibilityTests/AccessibilityProject/Sources/TargetA/RedundantFilePrivateComponents.swift b/Tests/AccessibilityTests/AccessibilityProject/Sources/TargetA/RedundantFilePrivateComponents.swift new file mode 100644 index 000000000..252d6ea2d --- /dev/null +++ b/Tests/AccessibilityTests/AccessibilityProject/Sources/TargetA/RedundantFilePrivateComponents.swift @@ -0,0 +1,47 @@ +// +// RedundantFilePrivateComponents.swift +// +// +// Created by Dan Wood on 4/18/23. +// + +import Foundation + +public class RedundantFilePrivateComponents { + + public init() { + let _ = SubclassThatShouldNotBeFilePrivate() + let _ = StructThatShouldNotBeFilePrivate() + let _ = EnumThatShouldNotBeFilePrivate.a + Self.aFuncThatShouldNotBeFilePrivate() + let _ = Self.aVarThatShouldNotBeFilePrivate + let _ = ClassThatShouldNotBeFilePrivate() + } + + // fileprivate is not ideal here; these could be private since they are not used outside of this class scope + + fileprivate class SubclassThatShouldNotBeFilePrivate {} + fileprivate struct StructThatShouldNotBeFilePrivate {} + fileprivate enum EnumThatShouldNotBeFilePrivate { + case a + } + fileprivate static func aFuncThatShouldNotBeFilePrivate() {} + fileprivate static var aVarThatShouldNotBeFilePrivate: Int = 0 +} + +// This can just be private +fileprivate class ClassThatShouldNotBeFilePrivate {} + +extension RedundantFilePrivateComponents { + + // These could also be private + + fileprivate class ExtensionSubclassThatShouldNotBeFilePrivate {} + fileprivate struct ExtensionStructThatShouldNotBeFilePrivate {} + fileprivate enum ExtensionEnumThatShouldNotBeFilePrivate { + case a + } + fileprivate static func aExtensionFuncThatShouldNotBeFilePrivate() {} + fileprivate static var aExtensionVarThatShouldNotBeFilePrivate: Int = 0 + +} diff --git a/Tests/AccessibilityTests/AccessibilityProject/Sources/TargetA/RedundantInternalComponents.swift b/Tests/AccessibilityTests/AccessibilityProject/Sources/TargetA/RedundantInternalComponents.swift new file mode 100644 index 000000000..618d5d039 --- /dev/null +++ b/Tests/AccessibilityTests/AccessibilityProject/Sources/TargetA/RedundantInternalComponents.swift @@ -0,0 +1,50 @@ +// +// RedundantInternalClassComponents.swift +// +// +// Created by Dan Wood on 4/18/23. +// + +import Foundation + +public class RedundantInternalClassComponents { + + public init() { + let _ = RedundantInternalSubclass() + let _ = RedundantInternalStruct() + let _ = RedundantInternalEnum.a + Self.aRedundantInternalFunc() + let _ = Self.aRedundantInternalVar + + let _ = ClassThatCanBePrivate() + } + + // Used only in this file, so they should be private. + + class RedundantInternalSubclass {} + struct RedundantInternalStruct {} + enum RedundantInternalEnum { + case a + } + static func aRedundantInternalFunc() {} + static var aRedundantInternalVar: Int = 0 +} + +extension RedundantInternalClassComponents { + + // Also should be private + + class ExtensionRedundantInternalSubclass {} + struct ExtensionRedundantInternalStruct {} + enum ExtensionRedundantInternalEnum { + case a + } + static func aExtensionRedundantInternalFunc() {} + static var aExtensionRedundantInternalVar: Int = 0 + +} + +class ClassThatCanBePrivate { + + static func FunctionThatShouldBePrivateInPrivateClass() {} +} diff --git a/Tests/AccessibilityTests/RedundantFilePrivateAccessibilityTest.swift b/Tests/AccessibilityTests/RedundantFilePrivateAccessibilityTest.swift new file mode 100644 index 000000000..94639de93 --- /dev/null +++ b/Tests/AccessibilityTests/RedundantFilePrivateAccessibilityTest.swift @@ -0,0 +1,68 @@ +import XCTest +import Shared +@testable import TestShared +@testable import PeripheryKit + +class RedundantFilePrivateAccessibilityTest: SourceGraphTestCase { + override static func setUp() { + super.setUp() + + configuration.targets = ["MainTarget", "TargetA", "TestTarget"] + + build(driver: SPMProjectDriver.self, projectPath: AccessibilityProjectPath) + index() + } + + func testRedundantFilePrivateTypes() { + assertRedundantFilePrivateAccessibility(.class("SubclassThatShouldNotBeFilePrivate")) + assertRedundantFilePrivateAccessibility(.struct("StructThatShouldNotBeFilePrivate")) + assertRedundantFilePrivateAccessibility(.enum("EnumThatShouldNotBeFilePrivate")) + assertRedundantFilePrivateAccessibility(.functionMethodStatic("aFuncThatShouldNotBeFilePrivate()")) + assertRedundantFilePrivateAccessibility(.varStatic("aVarThatShouldNotBeFilePrivate")) + } + + func testExtensionRedundantFilePrivateTypes() { + assertRedundantFilePrivateAccessibility(.class("ExtensionSubclassThatShouldNotBeFilePrivate")) + assertRedundantFilePrivateAccessibility(.struct("ExtensionStructThatShouldNotBeFilePrivate")) + assertRedundantFilePrivateAccessibility(.enum("ExtensionEnumThatShouldNotBeFilePrivate")) + assertRedundantFilePrivateAccessibility(.functionMethodStatic("aExtensionFuncThatShouldNotBeFilePrivate()")) + assertRedundantFilePrivateAccessibility(.varStatic("aExtensionVarThatShouldNotBeFilePrivate")) + } + + + func testNotRedundantFilePrivateTypes() { + assertNotRedundantFilePrivateAccessibility(.class("SubclassCorrectlyFilePrivate")) + assertNotRedundantFilePrivateAccessibility(.struct("StructCorrectlyFilePrivate")) + assertNotRedundantFilePrivateAccessibility(.enum("EnumCorrectlyFilePrivate")) + assertNotRedundantFilePrivateAccessibility(.functionMethodStatic("aFuncCorrectlyFilePrivate()")) + assertNotRedundantFilePrivateAccessibility(.varStatic("aVarCorrectlyFilePrivate")) + } + + func testExtensionNotRedundantFilePrivateTypes() { + assertNotRedundantFilePrivateAccessibility(.class("ExtensionSubclassCorrectlyFilePrivate")) + assertNotRedundantFilePrivateAccessibility(.struct("ExtensionStructCorrectlyFilePrivate")) + assertNotRedundantFilePrivateAccessibility(.enum("ExtensionEnumCorrectlyFilePrivate")) + assertNotRedundantFilePrivateAccessibility(.functionMethodStatic("aExtensionFuncCorrectlyFilePrivate()")) + assertNotRedundantFilePrivateAccessibility(.varStatic("aExtensionVarCorrectlyFilePrivate")) + } + +} + + +/* + + + + Periphery currently doesn't support `open` … "Open declarations are not yet implemented" + So we are looking for cases of: + - scope of fileprivate when it should be private; it's not used outside of the scope it's defined in + - scope of internal when it should be private/fileprivate; it's not used outside of the file it's defined in. + - Maybe OK to not suggest what the scope should be; if we fix to fileprivate then the next pass will warn about needing to be private + - scope of public when it should be otherwise — This is what the status quo does + + + + + + + */ diff --git a/Tests/AccessibilityTests/RedundantInternalAccessibilityTest.swift b/Tests/AccessibilityTests/RedundantInternalAccessibilityTest.swift new file mode 100644 index 000000000..5170dfe5b --- /dev/null +++ b/Tests/AccessibilityTests/RedundantInternalAccessibilityTest.swift @@ -0,0 +1,75 @@ +import XCTest +import Shared +@testable import TestShared +@testable import PeripheryKit + +class RedundantInternalAccessibilityTest: SourceGraphTestCase { + override static func setUp() { + super.setUp() + + configuration.targets = ["MainTarget", "TargetA", "TestTarget"] + + build(driver: SPMProjectDriver.self, projectPath: AccessibilityProjectPath) + index() + } + + func testRedundantInternalTypes() { + assertRedundantInternalAccessibility(.class("RedundantInternalSubclass")) + assertRedundantInternalAccessibility(.struct("RedundantInternalStruct")) + assertRedundantInternalAccessibility(.enum("RedundantInternalEnum")) + assertRedundantInternalAccessibility(.functionMethodStatic("aRedundantInternalFunc()")) + assertRedundantInternalAccessibility(.varStatic("aRedundantInternalVar")) + + assertRedundantInternalAccessibility(.class("ClassThatCanBePrivate")) + } + + func testExtensionRedundantInternalTypes() { + assertRedundantInternalAccessibility(.class("ExtensionRedundantInternalSubclass")) + assertRedundantInternalAccessibility(.struct("ExtensionRedundantInternalStruct")) + assertRedundantInternalAccessibility(.enum("ExtensionRedundantInternalEnum")) + assertRedundantInternalAccessibility(.functionMethodStatic("aExtensionRedundantInternalFunc()")) + assertRedundantInternalAccessibility(.varStatic("aExtensionRedundantInternalVar")) + } + + func testNotRedundantInternalTypes() { + assertNotRedundantInternalAccessibility(.class("NotRedundantInternalSubclass")) + assertNotRedundantInternalAccessibility(.struct("NotRedundantInternalStruct")) + assertNotRedundantInternalAccessibility(.enum("NotRedundantInternalEnum")) + assertNotRedundantInternalAccessibility(.functionMethodStatic("aNotRedundantInternalFunc()")) + assertNotRedundantInternalAccessibility(.varStatic("aNotRedundantInternalVar")) + + assertNotRedundantInternalAccessibility(.functionMethodStatic("protocolMethod()")) + assertNotRedundantInternalAccessibility(.functionMethodInstance("applicationDidFinishLaunching(_:)")) + + // Marking entire class as needing to be private so this component shouldn't be flagged + assertNotRedundantInternalAccessibility(.functionMethodStatic("FunctionThatShouldBePrivateInPrivateClass()")) + + } + + func testExtensionNotRedundantInternalTypes() { + assertNotRedundantInternalAccessibility(.class("ExtensionNotRedundantInternalSubclass")) + assertNotRedundantInternalAccessibility(.struct("ExtensionNotRedundantInternalStruct")) + assertNotRedundantInternalAccessibility(.enum("ExtensionNotRedundantInternalEnum")) + assertNotRedundantInternalAccessibility(.functionMethodStatic("aExtensionNotRedundantInternalFunc()")) + assertNotRedundantInternalAccessibility(.varStatic("aExtensionNotRedundantInternalVar")) + } +} + + +/* + + + + Periphery currently doesn't support `open` … "Open declarations are not yet implemented" + So we are looking for cases of: + - scope of Internal when it should be private; it's not used outside of the scope it's defined in + - scope of internal when it should be private/Internal; it's not used outside of the file it's defined in. + - Maybe OK to not suggest what the scope should be; if we fix to Internal then the next pass will warn about needing to be private + - scope of public when it should be otherwise — This is what the status quo does + + + + + + + */ diff --git a/Tests/Shared/SourceGraphTestCase.swift b/Tests/Shared/SourceGraphTestCase.swift index cbc551d50..de2a2cfaa 100644 --- a/Tests/Shared/SourceGraphTestCase.swift +++ b/Tests/Shared/SourceGraphTestCase.swift @@ -133,6 +133,55 @@ open class SourceGraphTestCase: XCTestCase { scopeStack.removeLast() } + func assertRedundantInternalAccessibility(_ description: DeclarationDescription, scopedAssertions: (() -> Void)? = nil, file: StaticString = #file, line: UInt = #line) { + guard let declaration = materialize(description, in: Self.allIndexedDeclarations, file: file, line: line) else { return } + + if !Self.graph.redundantInternalAccessibility.keys.contains(declaration) { + XCTFail("Expected declaration to have redundant internal accessibility: \(declaration)", file: file, line: line) + } + + scopeStack.append(.declaration(declaration)) + scopedAssertions?() + scopeStack.removeLast() + } + + func assertNotRedundantInternalAccessibility(_ description: DeclarationDescription, scopedAssertions: (() -> Void)? = nil, file: StaticString = #file, line: UInt = #line) { + guard let declaration = materialize(description, in: Self.allIndexedDeclarations, file: file, line: line) else { return } + + if Self.graph.redundantInternalAccessibility.keys.contains(declaration) { + XCTFail("Expected declaration to not have redundant internal accessibility: \(declaration)", file: file, line: line) + } + + scopeStack.append(.declaration(declaration)) + scopedAssertions?() + scopeStack.removeLast() + } + + func assertRedundantFilePrivateAccessibility(_ description: DeclarationDescription, scopedAssertions: (() -> Void)? = nil, file: StaticString = #file, line: UInt = #line) { + guard let declaration = materialize(description, in: Self.allIndexedDeclarations, file: file, line: line) else { return } + + if !Self.graph.redundantFilePrivateAccessibility.keys.contains(declaration) { + XCTFail("Expected declaration to have redundant fileprivate accessibility: \(declaration)", file: file, line: line) + } + + scopeStack.append(.declaration(declaration)) + scopedAssertions?() + scopeStack.removeLast() + } + + func assertNotRedundantFilePrivateAccessibility(_ description: DeclarationDescription, scopedAssertions: (() -> Void)? = nil, file: StaticString = #file, line: UInt = #line) { + guard let declaration = materialize(description, in: Self.allIndexedDeclarations, file: file, line: line) else { return } + + if Self.graph.redundantFilePrivateAccessibility.keys.contains(declaration) { + XCTFail("Expected declaration to not have redundant fileprivate accessibility: \(declaration)", file: file, line: line) + } + + scopeStack.append(.declaration(declaration)) + scopedAssertions?() + scopeStack.removeLast() + } + + func assertUsedParameter(_ name: String, file: StaticString = #file, line: UInt = #line) { let declaration = materialize(.varParameter(name), fail: false, file: file, line: line) diff --git a/Tests/XcodeTests/UIKitProject/UIKitProject/AppDelegate.swift b/Tests/XcodeTests/UIKitProject/UIKitProject/AppDelegate.swift index 958c13c15..996440425 100644 --- a/Tests/XcodeTests/UIKitProject/UIKitProject/AppDelegate.swift +++ b/Tests/XcodeTests/UIKitProject/UIKitProject/AppDelegate.swift @@ -18,8 +18,14 @@ class AppDelegate: UIResponder, UIApplicationDelegate { } func application(_ application: UIApplication, didDiscardSceneSessions sceneSessions: Set) { + foobar() // Called when the user discards a scene session. // If any sessions were discarded while the application was not running, this will be called shortly after application:didFinishLaunchingWithOptions. // Use this method to release any resources that were specific to the discarded scenes, as they will not return. } + + // Should be private + public func foobar() { + + } }