diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 3f423c240..247141704 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -125,6 +125,8 @@ jobs: run: ${{ env.swift_build }} - name: Scan run: ${{ env.periphery_scan }} ${{ matrix.baseline && format('--baseline baselines/{0}', matrix.baseline) || '' }} + - name: Clean SwiftPM cache + run: rm -rf ~/Library/Caches/org.swift.swiftpm - name: Test run: ${{ env.swift_test }} linux: @@ -172,5 +174,7 @@ jobs: run: ${{ env.swift_build }} - name: Scan run: ${{ env.periphery_scan }} --baseline baselines/linux.json + - name: Clean SwiftPM cache + run: rm -rf ~/.cache/org.swift.swiftpm - name: Test run: ${{ env.swift_test }} diff --git a/README.md b/README.md index 715c3bb04..0aefe5acb 100644 --- a/README.md +++ b/README.md @@ -28,6 +28,8 @@ - [Enumerations](#enumerations) - [Assign-only Properties](#assign-only-properties) - [Redundant Public Accessibility](#redundant-public-accessibility) + - [Redundant Internal Accessibility](#redundant-internal-accessibility) + - [Redundant Fileprivate Accessibility](#redundant-fileprivate-accessibility) - [Unused Imports](#unused-imports) - [Objective-C](#objective-c) - [Codable](#codable) @@ -292,6 +294,18 @@ Declarations that are marked `public` yet are not referenced from outside their This analysis can be disabled with `--disable-redundant-public-analysis`. +### Redundant Internal Accessibility + +Declarations that are marked `internal` (or are unmarked, since this is Swift's default access level), yet are not referenced outside the file they're defined in are identified as having redundant internal accessibility. In this scenario, the declaration could be marked `private` or `fileprivate`. Reducing the visibility of declarations — encapsulation — helps with code maintainability and can improve compilation performance. + +This analysis can be disabled with `--disable-redundant-internal-analysis`. + +### Redundant Fileprivate Accessibility + +Declarations that are marked `fileprivate` yet are not accessed from other types within the same file are identified as having redundant fileprivate accessibility. If a `fileprivate` declaration is only used within its own type, it should be marked `private` instead. Reducing the visibility of declarations helps with code maintainability and makes access boundaries clearer. + +This analysis can be disabled with `--disable-redundant-fileprivate-analysis`. + ### Unused Imports Periphery can only detect unused imports of targets it has scanned. It cannot detect unused imports of other targets because the Swift source files are unavailable and uses of `@_exported` cannot be observed. `@_exported` is problematic because it changes the public interface of a target such that the declarations exported by the target are no longer necessarily declared by the imported target. For example, the `Foundation` target exports `Dispatch`, among other targets. If any given source file imports `Foundation` and references `DispatchQueue` but no other declarations from `Foundation`, then the `Foundation` import cannot be removed as it would also make the `DispatchQueue` type unavailable. To avoid false positives, therefore, Periphery only detects unused imports of targets it has scanned. diff --git a/Sources/BUILD.bazel b/Sources/BUILD.bazel index 5210e93b1..c0a481d31 100644 --- a/Sources/BUILD.bazel +++ b/Sources/BUILD.bazel @@ -77,7 +77,10 @@ swift_library( "SourceGraph/Mutators/ProtocolConformanceReferenceBuilder.swift", "SourceGraph/Mutators/ProtocolExtensionReferenceBuilder.swift", "SourceGraph/Mutators/PubliclyAccessibleRetainer.swift", + "SourceGraph/Mutators/RedundantAccessibilityMarkerShared.swift", "SourceGraph/Mutators/RedundantExplicitPublicAccessibilityMarker.swift", + "SourceGraph/Mutators/RedundantFilePrivateAccessibilityMarker.swift", + "SourceGraph/Mutators/RedundantInternalAccessibilityMarker.swift", "SourceGraph/Mutators/RedundantProtocolMarker.swift", "SourceGraph/Mutators/ResultBuilderRetainer.swift", "SourceGraph/Mutators/StringInterpolationAppendInterpolationRetainer.swift", diff --git a/Sources/Configuration/Configuration.swift b/Sources/Configuration/Configuration.swift index 9d39fcc9b..a04c6759c 100644 --- a/Sources/Configuration/Configuration.swift +++ b/Sources/Configuration/Configuration.swift @@ -80,6 +80,15 @@ public final class Configuration { @Setting(key: "disable_redundant_public_analysis", defaultValue: false) public var disableRedundantPublicAnalysis: Bool + @Setting(key: "disable_redundant_internal_analysis", defaultValue: false) + public var disableRedundantInternalAnalysis: Bool + + @Setting(key: "disable_redundant_fileprivate_analysis", defaultValue: false) + public var disableRedundantFilePrivateAnalysis: Bool + + @Setting(key: "show_nested_redundant_accessibility", defaultValue: false) + public var showNestedRedundantAccessibility: Bool + @Setting(key: "disable_unused_import_analysis", defaultValue: false) public var disableUnusedImportAnalysis: Bool @@ -212,10 +221,11 @@ public final class Configuration { // MARK: - Private - lazy var settings: [any AbstractSetting] = [ + private lazy var settings: [any AbstractSetting] = [ $project, $schemes, $excludeTargets, $excludeTests, $indexExclude, $reportExclude, $reportInclude, $outputFormat, $retainPublic, $noRetainSPI, $retainFiles, $retainAssignOnlyProperties, $retainAssignOnlyPropertyTypes, $retainObjcAccessible, $retainObjcAnnotated, $retainUnusedProtocolFuncParams, $retainSwiftUIPreviews, $disableRedundantPublicAnalysis, + $disableRedundantInternalAnalysis, $disableRedundantFilePrivateAnalysis, $showNestedRedundantAccessibility, $disableUnusedImportAnalysis, $superfluousIgnoreComments, $retainUnusedImportedModules, $externalEncodableProtocols, $externalCodableProtocols, $externalTestCaseClasses, $verbose, $quiet, $color, $disableUpdateCheck, $strict, $indexStorePath, @@ -243,7 +253,7 @@ public final class Configuration { } } -protocol AbstractSetting { +private protocol AbstractSetting { associatedtype Value var key: String { get } diff --git a/Sources/Frontend/Commands/ScanCommand.swift b/Sources/Frontend/Commands/ScanCommand.swift index 1a988fc9c..e7f77169e 100644 --- a/Sources/Frontend/Commands/ScanCommand.swift +++ b/Sources/Frontend/Commands/ScanCommand.swift @@ -13,151 +13,160 @@ struct ScanCommand: ParsableCommand { ) @Argument(help: "Arguments following '--' will be passed to the underlying build tool, which is either 'swift build' or 'xcodebuild' depending on your project") - var buildArguments: [String] = defaultConfiguration.$buildArguments.defaultValue + private var buildArguments: [String] = defaultConfiguration.$buildArguments.defaultValue @Flag(help: "Enable guided setup") - var setup: Bool = defaultConfiguration.guidedSetup + private var setup: Bool = defaultConfiguration.guidedSetup @Option(help: "Path to the root directory of your project") - var projectRoot: FilePath = projectRootDefault + private var projectRoot: FilePath = projectRootDefault @Option(help: "Path to configuration file. By default Periphery will look for .periphery.yml in the current directory") - var config: FilePath? + private var config: FilePath? @Option(help: "Path to your project's .xcodeproj or .xcworkspace") - var project: FilePath? + private var project: FilePath? @Option(parsing: .upToNextOption, help: "Schemes to build. All targets built by these schemes will be scanned") - var schemes: [String] = defaultConfiguration.$schemes.defaultValue + private var schemes: [String] = defaultConfiguration.$schemes.defaultValue @Option(help: "Output format") - var format: OutputFormat = defaultConfiguration.$outputFormat.defaultValue + private var format: OutputFormat = defaultConfiguration.$outputFormat.defaultValue @Flag(help: "Exclude test targets from indexing") - var excludeTests: Bool = defaultConfiguration.$excludeTests.defaultValue + private var excludeTests: Bool = defaultConfiguration.$excludeTests.defaultValue @Option(parsing: .upToNextOption, help: "Targets to exclude from indexing") - var excludeTargets: [String] = defaultConfiguration.$excludeTargets.defaultValue + private var excludeTargets: [String] = defaultConfiguration.$excludeTargets.defaultValue @Option(parsing: .upToNextOption, help: "Source file globs to exclude from indexing") - var indexExclude: [String] = defaultConfiguration.$indexExclude.defaultValue + private var indexExclude: [String] = defaultConfiguration.$indexExclude.defaultValue @Option(parsing: .upToNextOption, help: "Source file globs to exclude from the results. Note that this option is purely cosmetic, these files will still be indexed") - var reportExclude: [String] = defaultConfiguration.$reportExclude.defaultValue + private var reportExclude: [String] = defaultConfiguration.$reportExclude.defaultValue @Option(parsing: .upToNextOption, help: "Source file globs to include in the results. This option supersedes '--report-exclude'. Note that this option is purely cosmetic, these files will still be indexed") - var reportInclude: [String] = defaultConfiguration.$reportInclude.defaultValue + private var reportInclude: [String] = defaultConfiguration.$reportInclude.defaultValue @Option(parsing: .upToNextOption, help: "Source file globs for which all containing declarations will be retained") - var retainFiles: [String] = defaultConfiguration.$retainFiles.defaultValue + private var retainFiles: [String] = defaultConfiguration.$retainFiles.defaultValue @Option(parsing: .upToNextOption, help: "Index store paths. Implies '--skip-build'") - var indexStorePath: [FilePath] = defaultConfiguration.$indexStorePath.defaultValue + private var indexStorePath: [FilePath] = defaultConfiguration.$indexStorePath.defaultValue @Flag(help: "Retain all public declarations, recommended for framework/library projects") - var retainPublic: Bool = defaultConfiguration.$retainPublic.defaultValue + private var retainPublic: Bool = defaultConfiguration.$retainPublic.defaultValue @Option(parsing: .upToNextOption, help: "Public SPIs (System Programming Interfaces) to check for unused code even when '--retain-public' is enabled") - var noRetainSPI: [String] = defaultConfiguration.$noRetainSPI.defaultValue + private var noRetainSPI: [String] = defaultConfiguration.$noRetainSPI.defaultValue @Flag(help: "Disable identification of redundant public accessibility") - var disableRedundantPublicAnalysis: Bool = defaultConfiguration.$disableRedundantPublicAnalysis.defaultValue + private var disableRedundantPublicAnalysis: Bool = defaultConfiguration.$disableRedundantPublicAnalysis.defaultValue + + @Flag(help: "Disable identification of redundant internal accessibility") + private var disableRedundantInternalAnalysis: Bool = defaultConfiguration.$disableRedundantInternalAnalysis.defaultValue + + @Flag(help: "Disable identification of redundant fileprivate accessibility") + private var disableRedundantFilePrivateAnalysis: Bool = defaultConfiguration.$disableRedundantFilePrivateAnalysis.defaultValue + + @Flag(help: "Show redundant internal/fileprivate accessibility warnings for nested declarations even when the containing type is already flagged") + private var showNestedRedundantAccessibility: Bool = defaultConfiguration.$showNestedRedundantAccessibility.defaultValue @Flag(help: "Disable identification of unused imports") - var disableUnusedImportAnalysis: Bool = defaultConfiguration.$disableUnusedImportAnalysis.defaultValue + private var disableUnusedImportAnalysis: Bool = defaultConfiguration.$disableUnusedImportAnalysis.defaultValue @Flag(inversion: .prefixedNo, help: "Report superfluous ignore comments") - var superfluousIgnoreComments: Bool = defaultConfiguration.$superfluousIgnoreComments.defaultValue + private var superfluousIgnoreComments: Bool = defaultConfiguration.$superfluousIgnoreComments.defaultValue @Option(parsing: .upToNextOption, help: "Names of unused imported modules to retain") - var retainUnusedImportedModules: [String] = defaultConfiguration.$retainUnusedImportedModules.defaultValue + private var retainUnusedImportedModules: [String] = defaultConfiguration.$retainUnusedImportedModules.defaultValue @Flag(help: "Retain properties that are assigned, but never used") - var retainAssignOnlyProperties: Bool = defaultConfiguration.$retainAssignOnlyProperties.defaultValue + private var retainAssignOnlyProperties: Bool = defaultConfiguration.$retainAssignOnlyProperties.defaultValue @Option(parsing: .upToNextOption, help: "Property types to retain if the property is assigned, but never read") - var retainAssignOnlyPropertyTypes: [String] = defaultConfiguration.$retainAssignOnlyPropertyTypes.defaultValue + private var retainAssignOnlyPropertyTypes: [String] = defaultConfiguration.$retainAssignOnlyPropertyTypes.defaultValue @Option(parsing: .upToNextOption, help: "Names of external protocols that inherit Encodable. Properties and CodingKey enums of types conforming to these protocols will be retained") - var externalEncodableProtocols: [String] = defaultConfiguration.$externalEncodableProtocols.defaultValue + private var externalEncodableProtocols: [String] = defaultConfiguration.$externalEncodableProtocols.defaultValue @Option(parsing: .upToNextOption, help: "Names of external protocols that inherit Codable. Properties and CodingKey enums of types conforming to these protocols will be retained") - var externalCodableProtocols: [String] = defaultConfiguration.$externalCodableProtocols.defaultValue + private var externalCodableProtocols: [String] = defaultConfiguration.$externalCodableProtocols.defaultValue @Option(parsing: .upToNextOption, help: "Names of XCTestCase subclasses that reside in external targets") - var externalTestCaseClasses: [String] = defaultConfiguration.$externalTestCaseClasses.defaultValue + private var externalTestCaseClasses: [String] = defaultConfiguration.$externalTestCaseClasses.defaultValue @Flag(help: "Retain declarations that are exposed to Objective-C implicitly by inheriting NSObject classes, or explicitly with the @objc and @objcMembers attributes") - var retainObjcAccessible: Bool = defaultConfiguration.$retainObjcAccessible.defaultValue + private var retainObjcAccessible: Bool = defaultConfiguration.$retainObjcAccessible.defaultValue @Flag(help: "Retain declarations that are exposed to Objective-C explicitly with the @objc and @objcMembers attributes") - var retainObjcAnnotated: Bool = defaultConfiguration.$retainObjcAnnotated.defaultValue + private var retainObjcAnnotated: Bool = defaultConfiguration.$retainObjcAnnotated.defaultValue @Flag(help: "Retain unused protocol function parameters, even if the parameter is unused in all conforming functions") - var retainUnusedProtocolFuncParams: Bool = defaultConfiguration.$retainUnusedProtocolFuncParams.defaultValue + private var retainUnusedProtocolFuncParams: Bool = defaultConfiguration.$retainUnusedProtocolFuncParams.defaultValue @Flag(help: "Retain SwiftUI previews") - var retainSwiftUIPreviews: Bool = defaultConfiguration.$retainSwiftUIPreviews.defaultValue + private var retainSwiftUIPreviews: Bool = defaultConfiguration.$retainSwiftUIPreviews.defaultValue @Flag(help: "Retain properties on Codable types (including Encodable and Decodable)") - var retainCodableProperties: Bool = defaultConfiguration.$retainCodableProperties.defaultValue + private var retainCodableProperties: Bool = defaultConfiguration.$retainCodableProperties.defaultValue @Flag(help: "Retain properties on Encodable types only") - var retainEncodableProperties: Bool = defaultConfiguration.$retainEncodableProperties.defaultValue + private var retainEncodableProperties: Bool = defaultConfiguration.$retainEncodableProperties.defaultValue @Flag(help: "Clean existing build artifacts before building") - var cleanBuild: Bool = defaultConfiguration.$cleanBuild.defaultValue + private var cleanBuild: Bool = defaultConfiguration.$cleanBuild.defaultValue @Flag(help: "Skip the project build step") - var skipBuild: Bool = defaultConfiguration.$skipBuild.defaultValue + private var skipBuild: Bool = defaultConfiguration.$skipBuild.defaultValue @Flag(help: "Skip schemes validation") - var skipSchemesValidation: Bool = defaultConfiguration.$skipSchemesValidation.defaultValue + private var skipSchemesValidation: Bool = defaultConfiguration.$skipSchemesValidation.defaultValue @Flag(help: "Output result paths relative to the current directory") - var relativeResults: Bool = defaultConfiguration.$relativeResults.defaultValue + private var relativeResults: Bool = defaultConfiguration.$relativeResults.defaultValue @Flag(help: "Exit with non-zero status if any unused code is found") - var strict: Bool = defaultConfiguration.$strict.defaultValue + private var strict: Bool = defaultConfiguration.$strict.defaultValue @Flag(help: "Disable checking for updates") - var disableUpdateCheck: Bool = defaultConfiguration.$disableUpdateCheck.defaultValue + private var disableUpdateCheck: Bool = defaultConfiguration.$disableUpdateCheck.defaultValue @Flag(help: "Enable verbose logging") - var verbose: Bool = defaultConfiguration.$verbose.defaultValue + private var verbose: Bool = defaultConfiguration.$verbose.defaultValue @Flag(help: "Only output results") - var quiet: Bool = defaultConfiguration.$quiet.defaultValue + private var quiet: Bool = defaultConfiguration.$quiet.defaultValue @Option(help: "Colored output mode") - var color: ColorOption = defaultConfiguration.$color.defaultValue + private var color: ColorOption = defaultConfiguration.$color.defaultValue @Flag(name: .customLong("no-color"), help: .hidden) - var noColor: Bool = false + private var noColor: Bool = false @Option(help: "JSON package manifest path (obtained using `swift package describe --type json` or manually)") - var jsonPackageManifestPath: FilePath? + private var jsonPackageManifestPath: FilePath? @Option(help: "Baseline file path used to filter results") - var baseline: FilePath? + private var baseline: FilePath? @Option(help: "Baseline file path where results are written. Pass the same path to '--baseline' in subsequent scans to exclude the results recorded in the baseline.") - var writeBaseline: FilePath? + private var writeBaseline: FilePath? @Option(help: "File path where formatted results are written.") - var writeResults: FilePath? + private var writeResults: FilePath? @Option(help: "Project configuration for non-Apple build systems") - var genericProjectConfig: FilePath? + private var genericProjectConfig: FilePath? @Flag(help: "Enable Bazel project mode") - var bazel: Bool = defaultConfiguration.$bazel.defaultValue + private var bazel: Bool = defaultConfiguration.$bazel.defaultValue @Option(help: "Filter pattern applied to the Bazel top-level targets query") - var bazelFilter: String? + private var bazelFilter: String? @Option(help: "Path to a global index store populated by Bazel. If provided, will be used instead of individual module stores.") - var bazelIndexStore: FilePath? + private var bazelIndexStore: FilePath? private static let defaultConfiguration = Configuration() @@ -190,6 +199,9 @@ struct ScanCommand: ParsableCommand { configuration.apply(\.$retainUnusedProtocolFuncParams, retainUnusedProtocolFuncParams) configuration.apply(\.$retainSwiftUIPreviews, retainSwiftUIPreviews) configuration.apply(\.$disableRedundantPublicAnalysis, disableRedundantPublicAnalysis) + configuration.apply(\.$disableRedundantInternalAnalysis, disableRedundantInternalAnalysis) + configuration.apply(\.$disableRedundantFilePrivateAnalysis, disableRedundantFilePrivateAnalysis) + configuration.apply(\.$showNestedRedundantAccessibility, showNestedRedundantAccessibility) configuration.apply(\.$disableUnusedImportAnalysis, disableUnusedImportAnalysis) configuration.apply(\.$superfluousIgnoreComments, superfluousIgnoreComments) configuration.apply(\.$retainUnusedImportedModules, retainUnusedImportedModules) diff --git a/Sources/Frontend/Project.swift b/Sources/Frontend/Project.swift index 60b694714..13909fbd4 100644 --- a/Sources/Frontend/Project.swift +++ b/Sources/Frontend/Project.swift @@ -6,7 +6,7 @@ import Shared import SystemPackage final class Project { - let kind: ProjectKind + private let kind: ProjectKind private let configuration: Configuration private let shell: Shell diff --git a/Sources/Frontend/main.swift b/Sources/Frontend/main.swift index 052f3bf39..9ee79aed5 100644 --- a/Sources/Frontend/main.swift +++ b/Sources/Frontend/main.swift @@ -3,7 +3,7 @@ import Foundation // When stdout is a pipe, enable line buffering so output is flushed after each // newline rather than block-buffered, ensuring timely output to the consumer. -var info = stat() +private var info = stat() fstat(STDOUT_FILENO, &info) if (info.st_mode & S_IFMT) == S_IFIFO { @@ -11,7 +11,7 @@ if (info.st_mode & S_IFMT) == S_IFIFO { setlinebuf(stderr) } -struct PeripheryCommand: ParsableCommand { +private struct PeripheryCommand: ParsableCommand { static let configuration = CommandConfiguration( commandName: "periphery", subcommands: [ diff --git a/Sources/Indexer/InfoPlistIndexer.swift b/Sources/Indexer/InfoPlistIndexer.swift index e6f9049c1..cb0f1addb 100644 --- a/Sources/Indexer/InfoPlistIndexer.swift +++ b/Sources/Indexer/InfoPlistIndexer.swift @@ -5,7 +5,7 @@ import SourceGraph import SystemPackage final class InfoPlistIndexer: Indexer { - enum PlistError: Error { + private enum PlistError: Error { case failedToParse(path: FilePath, underlyingError: Error) } diff --git a/Sources/Indexer/XCDataModelIndexer.swift b/Sources/Indexer/XCDataModelIndexer.swift index 00cc0496c..3d3a44a95 100644 --- a/Sources/Indexer/XCDataModelIndexer.swift +++ b/Sources/Indexer/XCDataModelIndexer.swift @@ -5,7 +5,7 @@ import SourceGraph import SystemPackage final class XCDataModelIndexer: Indexer { - enum XCDataModelError: Error { + private enum XCDataModelError: Error { case failedToParse(path: FilePath, underlyingError: Error) } diff --git a/Sources/Indexer/XibIndexer.swift b/Sources/Indexer/XibIndexer.swift index 0cf837be0..8c456c361 100644 --- a/Sources/Indexer/XibIndexer.swift +++ b/Sources/Indexer/XibIndexer.swift @@ -5,7 +5,7 @@ import SourceGraph import SystemPackage final class XibIndexer: Indexer { - enum XibError: Error { + private enum XibError: Error { case failedToParse(path: FilePath, underlyingError: Error) } diff --git a/Sources/Logger/Logger.swift b/Sources/Logger/Logger.swift index 4ff721a6e..d6afb5913 100644 --- a/Sources/Logger/Logger.swift +++ b/Sources/Logger/Logger.swift @@ -28,13 +28,13 @@ public enum LoggerColorMode: Sendable { } public struct Logger: Sendable { - let outputQueue: DispatchQueue - let quiet: Bool - let verbose: Bool - let colorMode: LoggerColorMode + private let outputQueue: DispatchQueue + private let quiet: Bool + private let verbose: Bool + private let colorMode: LoggerColorMode #if canImport(os) - let signposter = OSSignposter() + private let signposter = OSSignposter() #endif public var isColoredOutputEnabled: Bool { @@ -115,7 +115,7 @@ public struct Logger: Sendable { // MARK: - Private - func log(_ line: String, output: UnsafeMutablePointer) { + private func log(_ line: String, output: UnsafeMutablePointer) { _ = outputQueue.sync { fputs(line + "\n", output) } } @@ -132,8 +132,8 @@ public struct Logger: Sendable { } public struct ContextualLogger: Sendable { - let logger: Logger - let context: String + fileprivate let logger: Logger + fileprivate let context: String public func contextualized(with innerContext: String) -> ContextualLogger { logger.contextualized(with: "\(context):\(innerContext)") @@ -154,8 +154,8 @@ public struct ContextualLogger: Sendable { #if canImport(os) public struct SignpostInterval { - let name: StaticString - let state: OSSignpostIntervalState + fileprivate let name: StaticString + fileprivate let state: OSSignpostIntervalState } #else public struct SignpostInterval { diff --git a/Sources/PeripheryKit/Results/OutputFormatter.swift b/Sources/PeripheryKit/Results/OutputFormatter.swift index 33b861632..e12c6c0ec 100644 --- a/Sources/PeripheryKit/Results/OutputFormatter.swift +++ b/Sources/PeripheryKit/Results/OutputFormatter.swift @@ -33,6 +33,10 @@ extension OutputFormatter { "redundantProtocol" case .redundantPublicAccessibility: "redundantPublicAccessibility" + case .redundantInternalAccessibility: + "redundantInternalAccessibility" + case .redundantFilePrivateAccessibility: + "redundantFilePrivateAccessibility" case .superfluousIgnoreCommand: "superfluousIgnoreCommand" } @@ -66,6 +70,12 @@ extension OutputFormatter { case let .redundantPublicAccessibility(modules): let modulesJoined = modules.sorted().joined(separator: ", ") description += "Redundant public accessibility for \(kindDisplayName) '\(name)' (not used outside of \(modulesJoined))" + case let .redundantInternalAccessibility(_, suggestedAccessibility): + let accessibilityText = suggestedAccessibility?.rawValue ?? "private/fileprivate" + description += "Redundant internal accessibility for \(kindDisplayName) '\(name)' (not used outside of file; can be \(accessibilityText))" + case let .redundantFilePrivateAccessibility(_, containingTypeName): + let context = containingTypeName.map { "only used within \($0)" } ?? "not used outside of file" + description += "Redundant fileprivate accessibility for \(kindDisplayName) '\(name)' (\(context); can be private)" case .superfluousIgnoreCommand: description += "Superfluous ignore comment for \(kindDisplayName) '\(name)' (declaration is referenced and should not be ignored)" } diff --git a/Sources/PeripheryKit/ScanResult.swift b/Sources/PeripheryKit/ScanResult.swift index e558270db..935080816 100644 --- a/Sources/PeripheryKit/ScanResult.swift +++ b/Sources/PeripheryKit/ScanResult.swift @@ -7,6 +7,8 @@ public struct ScanResult { case assignOnlyProperty case redundantProtocol(references: Set, inherited: Set) case redundantPublicAccessibility(modules: Set) + case redundantInternalAccessibility(files: Set, suggestedAccessibility: Accessibility?) + case redundantFilePrivateAccessibility(files: Set, containingTypeName: String?) case superfluousIgnoreCommand } diff --git a/Sources/PeripheryKit/ScanResultBuilder.swift b/Sources/PeripheryKit/ScanResultBuilder.swift index 7b63e17f7..5fd0c1813 100644 --- a/Sources/PeripheryKit/ScanResultBuilder.swift +++ b/Sources/PeripheryKit/ScanResultBuilder.swift @@ -10,6 +10,8 @@ public enum ScanResultBuilder { .union(graph.unusedModuleImports) 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.flatMap { removableDeclaration in var extensionResults = [ScanResult]() @@ -41,6 +43,12 @@ public enum 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(files: $0.1.files, suggestedAccessibility: $0.1.suggestedAccessibility)) + } + let annotatedRedundantFilePrivateAccessibility: [ScanResult] = redundantFilePrivateAccessibility.map { + .init(declaration: $0.0, annotation: .redundantFilePrivateAccessibility(files: $0.1.files, containingTypeName: $0.1.containingTypeName)) + } let annotatedSuperfluousIgnoreCommands: [ScanResult] = { guard configuration.superfluousIgnoreComments else { return [] } @@ -64,6 +72,8 @@ public enum ScanResultBuilder { annotatedAssignOnlyProperties + annotatedRedundantProtocols + annotatedRedundantPublicAccessibility + + annotatedRedundantInternalAccessibility + + annotatedRedundantFilePrivateAccessibility + annotatedSuperfluousIgnoreCommands return allAnnotatedDeclarations diff --git a/Sources/ProjectDrivers/GenericProjectDriver.swift b/Sources/ProjectDrivers/GenericProjectDriver.swift index 6807e3643..4a166b95b 100644 --- a/Sources/ProjectDrivers/GenericProjectDriver.swift +++ b/Sources/ProjectDrivers/GenericProjectDriver.swift @@ -7,7 +7,7 @@ import SwiftIndexStore import SystemPackage public final class GenericProjectDriver { - struct GenericConfig: Decodable { + private struct GenericConfig: Decodable { let indexstores: Set let plists: Set let xibs: Set diff --git a/Sources/ProjectDrivers/SPMProjectDriver.swift b/Sources/ProjectDrivers/SPMProjectDriver.swift index 5b54afdfe..f95049035 100644 --- a/Sources/ProjectDrivers/SPMProjectDriver.swift +++ b/Sources/ProjectDrivers/SPMProjectDriver.swift @@ -20,7 +20,7 @@ public final class SPMProjectDriver { self.init(pkg: pkg, configuration: configuration, logger: logger) } - init(pkg: SPM.Package, configuration: Configuration, logger: Logger) { + private init(pkg: SPM.Package, configuration: Configuration, logger: Logger) { self.pkg = pkg self.configuration = configuration self.logger = logger diff --git a/Sources/ProjectDrivers/XcodeProjectDriver.swift b/Sources/ProjectDrivers/XcodeProjectDriver.swift index 943fe363c..058708c8f 100644 --- a/Sources/ProjectDrivers/XcodeProjectDriver.swift +++ b/Sources/ProjectDrivers/XcodeProjectDriver.swift @@ -79,7 +79,7 @@ ) } - init( + private init( logger: Logger, configuration: Configuration, xcodebuild: Xcodebuild, diff --git a/Sources/Shared/JobPool.swift b/Sources/Shared/JobPool.swift index ae527d431..2d2b38eaf 100644 --- a/Sources/Shared/JobPool.swift +++ b/Sources/Shared/JobPool.swift @@ -2,7 +2,7 @@ import Foundation import Synchronization public struct JobPool { - let jobs: [Job] + private let jobs: [Job] public init(jobs: [Job]) { self.jobs = jobs diff --git a/Sources/Shared/SetupGuide.swift b/Sources/Shared/SetupGuide.swift index bc10dde2a..35790554d 100644 --- a/Sources/Shared/SetupGuide.swift +++ b/Sources/Shared/SetupGuide.swift @@ -26,7 +26,7 @@ open class SetupGuideHelpers { self.logger = logger } - func display(options: [String]) { + private func display(options: [String]) { let maxPaddingCount = String(options.count).count for (index, option) in options.enumerated() { diff --git a/Sources/Shared/Shell.swift b/Sources/Shared/Shell.swift index 0f7c1667a..0d89148ce 100644 --- a/Sources/Shared/Shell.swift +++ b/Sources/Shared/Shell.swift @@ -2,7 +2,7 @@ import Foundation import Logger import Synchronization -final class ShellProcessStore: Sendable { +private final class ShellProcessStore: Sendable { private let processes = Mutex>([]) func interruptRunning() { diff --git a/Sources/Shared/SwiftVersion.swift b/Sources/Shared/SwiftVersion.swift index 4a4210fb4..e1ef6bee1 100644 --- a/Sources/Shared/SwiftVersion.swift +++ b/Sources/Shared/SwiftVersion.swift @@ -2,7 +2,7 @@ import Extensions import Foundation public struct SwiftVersion { - static let minimumVersion = "5.10" + private static let minimumVersion = "5.10" public let version: VersionString public let fullVersion: String diff --git a/Sources/SourceGraph/Elements/DeclarationAttribute.swift b/Sources/SourceGraph/Elements/DeclarationAttribute.swift index d8e1b1b93..832cb976b 100644 --- a/Sources/SourceGraph/Elements/DeclarationAttribute.swift +++ b/Sources/SourceGraph/Elements/DeclarationAttribute.swift @@ -1,6 +1,6 @@ public struct DeclarationAttribute: Hashable, CustomStringConvertible { let name: String - let arguments: String? + private let arguments: String? public init(name: String, arguments: String?) { self.name = name diff --git a/Sources/SourceGraph/Elements/Reference.swift b/Sources/SourceGraph/Elements/Reference.swift index f1f29f893..4497807e6 100644 --- a/Sources/SourceGraph/Elements/Reference.swift +++ b/Sources/SourceGraph/Elements/Reference.swift @@ -75,7 +75,7 @@ extension Reference: CustomStringConvertible { "Reference(\(descriptionParts.joined(separator: ", ")))" } - var descriptionParts: [String] { + private var descriptionParts: [String] { let formattedName = name != nil ? "'\(name!)'" : "nil" return [kind.rawValue, declarationKind.rawValue, formattedName, "'\(usr)'", role.rawValue, location.shortDescription] diff --git a/Sources/SourceGraph/Mutators/EnumCaseReferenceBuilder.swift b/Sources/SourceGraph/Mutators/EnumCaseReferenceBuilder.swift index 8a74e4760..359aeb661 100644 --- a/Sources/SourceGraph/Mutators/EnumCaseReferenceBuilder.swift +++ b/Sources/SourceGraph/Mutators/EnumCaseReferenceBuilder.swift @@ -38,7 +38,7 @@ final class EnumCaseReferenceBuilder: SourceGraphMutator { // MARK: - Private - func isRawRepresentable(_ enumDeclaration: Declaration) -> Bool { + private func isRawRepresentable(_ enumDeclaration: Declaration) -> Bool { // If the enum has a related struct it's very likely to be raw representable, // and thus is dynamic in nature. diff --git a/Sources/SourceGraph/Mutators/RedundantAccessibilityMarkerShared.swift b/Sources/SourceGraph/Mutators/RedundantAccessibilityMarkerShared.swift new file mode 100644 index 000000000..894de5361 --- /dev/null +++ b/Sources/SourceGraph/Mutators/RedundantAccessibilityMarkerShared.swift @@ -0,0 +1,55 @@ +// Shared utilities for redundant accessibility analysis mutators. + +extension Declaration { + /// Checks if this declaration is referenced outside its defining file. + /// This is a common check used by multiple accessibility markers to determine + /// if a declaration needs file-level or module-level accessibility. + func isReferencedOutsideFile(graph: SourceGraph) -> Bool { + graph.references(to: self).map(\.location.file).contains { $0 != location.file } + } + + /// Generic recursive descendent declaration finder with filtering. + /// Recursively traverses the declaration tree and returns all descendants matching the predicate. + /// Used by all accessibility markers to find declarations with specific accessibility levels. + func descendentDeclarations(matching predicate: (Declaration) -> Bool) -> Set { + let matchingDeclarations = declarations.filter(predicate) + return matchingDeclarations + .flatMapSet { $0.descendentDeclarations(matching: predicate) } + .union(matchingDeclarations) + } + + /// Checks if any ancestor declaration is marked as redundant in the given accessibility map. + /// Used by accessibility markers to suppress nested warnings when a containing type is already flagged. + /// This avoids redundant warnings since fixing the parent's accessibility fixes the children too. + func isAnyAncestorMarked(in accessibilityMap: [Declaration: Any]) -> Bool { + var current = parent + var visited: Set = [] + + while let currentParent = current { + guard !visited.contains(currentParent) else { + return false + } + + visited.insert(currentParent) + + if accessibilityMap[currentParent] != nil { + return true + } + current = currentParent.parent + } + return false + } + + /// Counts the number of ancestors for this declaration. + /// Used for sorting declarations by depth to ensure parents are marked before children, + /// which is important for nested redundant accessibility suppression logic. + var ancestorCount: Int { + var count = 0 + var current = parent + while current != nil { + count += 1 + current = current?.parent + } + return count + } +} diff --git a/Sources/SourceGraph/Mutators/RedundantExplicitPublicAccessibilityMarker.swift b/Sources/SourceGraph/Mutators/RedundantExplicitPublicAccessibilityMarker.swift index 5e6f71eaa..5f50ade08 100644 --- a/Sources/SourceGraph/Mutators/RedundantExplicitPublicAccessibilityMarker.swift +++ b/Sources/SourceGraph/Mutators/RedundantExplicitPublicAccessibilityMarker.swift @@ -184,7 +184,8 @@ final class RedundantExplicitPublicAccessibilityMarker: SourceGraphMutator { } private func descendentPublicDeclarations(from decl: Declaration) -> Set { - let publicDeclarations = decl.declarations.filter { !$0.isImplicit && $0.accessibility.isExplicitly(.public) } - return publicDeclarations.flatMapSet { descendentPublicDeclarations(from: $0) }.union(publicDeclarations) + decl.descendentDeclarations(matching: { + !$0.isImplicit && $0.accessibility.isExplicitly(.public) + }) } } diff --git a/Sources/SourceGraph/Mutators/RedundantFilePrivateAccessibilityMarker.swift b/Sources/SourceGraph/Mutators/RedundantFilePrivateAccessibilityMarker.swift new file mode 100644 index 000000000..aac807fa0 --- /dev/null +++ b/Sources/SourceGraph/Mutators/RedundantFilePrivateAccessibilityMarker.swift @@ -0,0 +1,185 @@ +import Configuration +import Shared + +/// Identifies declarations explicitly marked `fileprivate` that don't actually need file-level access. +/// +/// Swift's `fileprivate` exists specifically to allow access from other types within the same file. +/// If a `fileprivate` declaration is only accessed within its own type (not from other types in +/// the same file), it should be marked `private` instead. +/// +/// This mutator is more complex than RedundantInternalAccessibilityMarker because it must: +/// - Distinguish between access from the same type vs. different types in the same file +/// - Handle extensions of types (both same-file and cross-file extensions) +/// - Walk the type hierarchy to find the top-level containing type for comparison +/// +/// The key insight: `private` and `fileprivate` differ in that `private` is accessible only within +/// the declaration and its extensions in the same file, while `fileprivate` is accessible from +/// anywhere in the same file. +final class RedundantFilePrivateAccessibilityMarker: SourceGraphMutator { + private let graph: SourceGraph + private let configuration: Configuration + + required init(graph: SourceGraph, configuration: Configuration, swiftVersion _: SwiftVersion) { + 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(\.kind.isExtensionKind) + + for decl in nonExtensionKinds { + try validate(decl) + } + + for decl in extensionKinds { + try validateExtension(decl) + } + } + + // MARK: - Private + + private func validate(_ decl: Declaration) throws { + if decl.accessibility.isExplicitly(.fileprivate) { + if !graph.isRetained(decl), + !decl.isReferencedOutsideFile(graph: graph), + !isReferencedFromDifferentTypeInSameFile(decl) + { + mark(decl) + } + } + + // Always check descendants, even if parent is not redundant. + // + // A parent declaration may be used outside its file (making it not redundant), + // while still having child declarations that are only used within the same file + // (making those children redundant). For example, a class used cross-file may have + // a fileprivate property only referenced within the same file - that property should + // be flagged as redundant even though the parent class is not. + markExplicitFilePrivateDescendentDeclarations(from: decl) + } + + private func validateExtension(_ decl: Declaration) throws { + if decl.accessibility.isExplicitly(.fileprivate) { + if let extendedDecl = try? graph.extendedDeclaration(forExtension: decl), + graph.redundantFilePrivateAccessibility.keys.contains(extendedDecl) + { + mark(decl) + } + } + } + + private func mark(_ decl: Declaration) { + guard !graph.isRetained(decl) else { return } + + // Unless explicitly requested, skip marking nested declarations when an ancestor is already marked. + // This avoids redundant warnings since fixing the parent's accessibility fixes the children too. + if !configuration.showNestedRedundantAccessibility, + decl.isAnyAncestorMarked(in: graph.redundantFilePrivateAccessibility) + { + return + } + + let containingTypeName = containingTypeName(for: decl) + graph.markRedundantFilePrivateAccessibility(decl, file: decl.location.file, containingTypeName: containingTypeName) + } + + private func markExplicitFilePrivateDescendentDeclarations(from decl: Declaration) { + // Sort descendants by their depth to ensure parents are marked before children. + // This is important for the nested redundant accessibility suppression logic. + let descendants = descendentFilePrivateDeclarations(from: decl).sorted { decl1, decl2 in + decl1.ancestorCount < decl2.ancestorCount + } + + for descDecl in descendants { + if !graph.isRetained(descDecl), + !descDecl.isReferencedOutsideFile(graph: graph), + !isReferencedFromDifferentTypeInSameFile(descDecl) + { + mark(descDecl) + } + } + } + + private func descendentFilePrivateDeclarations(from decl: Declaration) -> Set { + decl.descendentDeclarations(matching: { + !$0.isImplicit && $0.accessibility.isExplicitly(.fileprivate) + }) + } + + /// Finds the top-level type declaration by walking up the parent chain. + /// Returns the outermost type that contains the given declaration. + private func topLevelType(of decl: Declaration) -> Declaration? { + let baseTypeKinds: Set = [.class, .struct, .enum, .protocol] + let typeKinds = baseTypeKinds.union(Declaration.Kind.extensionKinds) + let ancestors = [decl] + Array(decl.ancestralDeclarations) + return ancestors.last { typeDecl in + guard typeKinds.contains(typeDecl.kind) else { return false } + guard let parent = typeDecl.parent else { return true } + + return !typeKinds.contains(parent.kind) + } + } + + /// Gets the logical type for comparison purposes. + /// For extensions of types in the SAME FILE, treats the extension as the extended type. + /// For extensions of types in DIFFERENT FILES (like extending external types), + /// treats the extension as its own distinct type for the purpose of this file. + private func logicalType(of decl: Declaration, inFile file: SourceFile) -> Declaration? { + if decl.kind.isExtensionKind { + if let extendedDecl = try? graph.extendedDeclaration(forExtension: decl), + extendedDecl.location.file == file + { + return extendedDecl + } + return decl + } + return decl + } + + /// Extracts a display name for the containing type of a declaration. + /// + /// Returns a string like "class Foo" or "struct Bar" that identifies the type + /// containing the declaration. Returns nil for top-level declarations. + private func containingTypeName(for decl: Declaration) -> String? { + guard let topLevel = topLevelType(of: decl) else { return nil } + guard let name = topLevel.name else { return nil } + + return "\(topLevel.kind.displayName) \(name)" + } + + /// Checks if a declaration is referenced from a different type in the same file. + /// Returns true if any same-file reference comes from a different logical type, + /// indicating that fileprivate access is necessary. + /// + /// Even for top-level declarations, private and fileprivate are different: + /// - private: only accessible within the declaration itself and its extensions in the same file + /// - fileprivate: accessible from anywhere in the same file + private func isReferencedFromDifferentTypeInSameFile(_ decl: Declaration) -> Bool { + let file = decl.location.file + let sameFileReferences = graph.references(to: decl).filter { $0.location.file == file } + + guard let declTopLevel = topLevelType(of: decl) else { + return false + } + + let declLogicalType = logicalType(of: declTopLevel, inFile: file) + + for ref in sameFileReferences { + guard let refParent = ref.parent else { continue } + guard let refTopLevel = topLevelType(of: refParent) else { + // Reference from a free function or top-level code — no containing type. + return true + } + + let refLogicalType = logicalType(of: refTopLevel, inFile: file) + + if declLogicalType !== refLogicalType { + return true + } + } + return false + } +} diff --git a/Sources/SourceGraph/Mutators/RedundantInternalAccessibilityMarker.swift b/Sources/SourceGraph/Mutators/RedundantInternalAccessibilityMarker.swift new file mode 100644 index 000000000..942441bc1 --- /dev/null +++ b/Sources/SourceGraph/Mutators/RedundantInternalAccessibilityMarker.swift @@ -0,0 +1,836 @@ +import Configuration +import Shared + +/// Identifies `internal` declarations (implicit, or explicitly marked) that are not referenced outside +/// the file they're defined in. +/// +/// Since `internal` is Swift's default access level, declarations that are only used within +/// their defining file should be marked `private` or `fileprivate` instead. This improves +/// encapsulation and can help with compilation performance. +/// +/// This mutator follows the same pattern as RedundantPublicAccessibilityMarker but checks +/// for file-scoped usage instead of module-scoped usage. +final class RedundantInternalAccessibilityMarker: SourceGraphMutator { + private let graph: SourceGraph + private let configuration: Configuration + + required init(graph: SourceGraph, configuration: Configuration, swiftVersion _: SwiftVersion) { + 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(\.kind.isExtensionKind) + + for decl in nonExtensionKinds { + try validate(decl) + } + + for decl in extensionKinds { + try validateExtension(decl) + } + } + + // MARK: - Private + + private func validate(_ decl: Declaration) throws { + if decl.accessibility.value == .internal { + if !graph.isRetained(decl), !shouldSkipMarking(decl) { + let isReferencedOutside = decl.isReferencedOutsideFile(graph: graph) + if !isReferencedOutside, !isTransitivelyExposedOutsideFile(decl) { + mark(decl) + } + } + } + + // Always check descendants, even if parent is not redundant. + // + // A parent declaration may be used outside its file (making it not redundant), + // while still having child declarations that are only used within the same file + // (making those children redundant). For example, a class used cross-file may have + // an internal property only referenced within the same file - that property should + // be flagged as redundant even though the parent class is not. + markInternalDescendentDeclarations(from: decl) + } + + private func validateExtension(_ decl: Declaration) throws { + if decl.accessibility.value == .internal { + if let extendedDecl = try? graph.extendedDeclaration(forExtension: decl), + graph.redundantInternalAccessibility.keys.contains(extendedDecl) + { + mark(decl) + } + } + } + + private func mark(_ decl: Declaration) { + guard !graph.isRetained(decl) else { return } + + // Unless explicitly requested, skip marking nested declarations when an ancestor is already marked. + // This avoids redundant warnings since fixing the parent's accessibility fixes the children too. + if !configuration.showNestedRedundantAccessibility, + decl.isAnyAncestorMarked(in: graph.redundantInternalAccessibility) + { + return + } + + // Determine the suggested accessibility level. + // If the declaration is referenced from different types in the same file, + // it needs fileprivate. Otherwise, private is sufficient. + // Also check transitive exposure: if the type is used as return/parameter type of a + // function called from a different type in the same file, it needs fileprivate. + // Additionally, types used in protocol requirement signatures need fileprivate even + // at top level (private would make them inaccessible from the protocol method). + let isTopLevel = decl.parent == nil + let needsFileprivate = isReferencedFromDifferentTypeInSameFile(decl) || + isTransitivelyExposedFromDifferentTypeInSameFile(decl) || + isUsedInProtocolRequirementSignature(decl) + + // For top-level declarations where private and fileprivate would both work, + // we pass nil to indicate the ambiguity. But if fileprivate is specifically needed + // (e.g., the type is used in a protocol requirement signature), we suggest fileprivate. + let suggestedAccessibility: Accessibility? = if isTopLevel, !needsFileprivate { + nil + } else { + needsFileprivate ? .fileprivate : .private + } + + // Check if the parent's accessibility already constrains this member. + // If the parent is `private`, the member is already effectively `private`. + // If the parent is `fileprivate` and we would suggest `fileprivate`, it's already constrained. + // Marking these would be misleading since changing them would actually increase visibility. + if let maxAccessibility = effectiveMaximumAccessibility(for: decl), + let suggestedAccessibility + { + let accessibilityOrder: [Accessibility] = [.private, .fileprivate, .internal, .public, .open] + let maxIndex = accessibilityOrder.firstIndex(of: maxAccessibility) ?? 0 + let suggestedIndex = accessibilityOrder.firstIndex(of: suggestedAccessibility) ?? 0 + + if suggestedIndex >= maxIndex { + return + } + } + + graph.markRedundantInternalAccessibility(decl, file: decl.location.file, suggestedAccessibility: suggestedAccessibility) + } + + private func markInternalDescendentDeclarations(from decl: Declaration) { + // Sort descendants by their depth to ensure parents are marked before children. + // This is important for the nested redundant accessibility suppression logic. + let descendants = descendentInternalDeclarations(from: decl).sorted { decl1, decl2 in + decl1.ancestorCount < decl2.ancestorCount + } + + for descDecl in descendants { + if !graph.isRetained(descDecl), !shouldSkipMarking(descDecl) { + let isReferencedOutside = descDecl.isReferencedOutsideFile(graph: graph) + if !isReferencedOutside, !isTransitivelyExposedOutsideFile(descDecl) { + mark(descDecl) + } + } + } + } + + /// Determines if a declaration should be skipped from redundant internal marking. + /// + /// Declarations are skipped if: + /// - They should be skipped from all accessibility analysis (generic type params, implicit decls) + /// - They are protocol requirements (must maintain accessibility for protocol conformance) + /// - They are part of a property wrapper's API (must be accessible to wrapper users) + /// - They are struct stored properties used in an implicit memberwise initializer + /// - They are referenced by a `#Preview` macro expansion (when retainSwiftUIPreviews is enabled) + private func shouldSkipMarking(_ decl: Declaration) -> Bool { + if shouldSkipAccessibilityAnalysis(for: decl) { + return true + } + + if isProtocolRequirement(decl) { + return true + } + + if isPropertyWrapperMember(decl) { + return true + } + + if isStructMemberwiseInitProperty(decl) { + return true + } + + if isUsedInExternalProtocolRequirementSignature(decl) { + return true + } + + // Check if type is constrained by same-file type usage + if isConstrainedBySameFileTypeUsage(decl) { + return true + } + + // Skip declarations referenced by #Preview macro expansions + if isReferencedByPreviewMacro(decl) { + return true + } + + return false + } + + private func descendentInternalDeclarations(from decl: Declaration) -> Set { + decl.descendentDeclarations(matching: { + $0.accessibility.value == .internal + }) + } + + // MARK: - Internal Accessibility Analysis Helpers + + /// Determines if a declaration should be skipped from accessibility analysis entirely. + /// + /// This helper is specific to internal accessibility analysis, checking conditions + /// that make a declaration ineligible for redundant internal marking. + private func shouldSkipAccessibilityAnalysis(for decl: Declaration) -> Bool { + // Generic type parameters must match their container's accessibility. + if decl.kind == .genericTypeParam { return true } + + // Skip implicit (compiler-generated) declarations. + if decl.isImplicit { return true } + + // Deinitializers cannot have explicit access modifiers in Swift. + if decl.kind == .functionDestructor { return true } + + // Enum cases cannot have explicit access modifiers in Swift. + // They inherit the accessibility of their containing enum. + if decl.kind == .enumelement { return true } + + // Override methods must be at least as accessible as what they override. + if decl.isOverride { return true } + + // Declarations with @usableFromInline must remain internal (or package). + // This attribute allows internal declarations to be inlined into client code, + // requiring them to maintain internal visibility. + if decl.attributes.contains(where: { $0.name == "usableFromInline" }) { + return true + } + + return false + } + + /// Checks if a declaration is a protocol requirement or protocol conformance. + /// + /// Protocol requirements must maintain sufficient accessibility to fulfill the protocol + /// contract, even if only referenced within the same file. This is critical for internal + /// accessibility analysis to avoid marking protocol implementations as redundant. + private func isProtocolRequirement(_ decl: Declaration) -> Bool { + // Case 1: Direct protocol requirement - parent is a protocol. + if let parent = decl.parent, parent.kind == .protocol { + return true + } + + // Case 2: Protocol conformance - this declaration implements a protocol requirement. + // + // When a type conforms to a protocol, Swift's indexer creates "related" references + // from the conforming declaration to the protocol requirement. If this declaration + // has any related references pointing to protocol members with matching names, + // it's implementing a protocol requirement. + let relatedReferences = graph.references(to: decl).filter { $0.kind == .related } + for ref in relatedReferences { + if let protocolDecl = graph.declaration(withUsr: ref.usr), + protocolDecl.kind.isProtocolMemberKind || protocolDecl.kind == .associatedtype + { + return true + } + } + + // Case 3: Check for .related references FROM this declaration to protocol members. + // This covers both internal AND external protocol conformances. + for ref in decl.related where ref.declarationKind.isProtocolMemberConformingKind { + if let referencedDecl = graph.declaration(withUsr: ref.usr) { + // Internal protocol: verify the referenced declaration's parent is a protocol. + if let referencedParent = referencedDecl.parent, + referencedParent.kind == .protocol + { + return true + } + } else if ref.name == decl.name { + // External protocol: the declaration doesn't exist in our graph, + // but the indexer created a .related reference with a protocol member kind + // AND the names match. This means this declaration implements an external + // protocol requirement. + return true + } + } + + return false + } + + /// Checks if a declaration is part of a property wrapper's public API. + /// + /// Property wrappers require certain members to be accessible even for internal + /// accessibility analysis. This prevents marking essential property wrapper components + /// as redundant when they're only referenced within the same file. + private func isPropertyWrapperMember(_ decl: Declaration) -> Bool { + guard let parent = decl.parent else { return false } + + // Check if parent type has @propertyWrapper attribute. + let isPropertyWrapper = parent.attributes.contains { $0.name == "propertyWrapper" } + guard isPropertyWrapper else { return false } + + // Property wrapper initializers are part of the API. + if decl.kind == .functionConstructor { return true } + + // wrappedValue and projectedValue are part of the API. + if decl.kind == .varInstance { + let propertyWrapperSpecialProperties = ["wrappedValue", "projectedValue"] + if propertyWrapperSpecialProperties.contains(decl.name ?? "") { + return true + } + } + + // Typealiases used in method/init signatures must be accessible. + // If this is a typealias and it's referenced by a function/init in the same + // property wrapper type, it's part of the API. + if decl.kind == .typealias { + let siblings = parent.declarations + let hasFunctionReference = siblings.contains { sibling in + sibling.kind.isFunctionKind && sibling.references.contains { ref in + ref.declarationKind == .typealias && decl.usrs.contains(ref.usr) + } + } + if hasFunctionReference { + return true + } + } + + return false + } + + /// Checks if a declaration is a stored property that's part of a struct's implicit memberwise + /// initializer AND that initializer is used (either from outside the file OR from within + /// the same file when the struct must remain internal). + private func isStructMemberwiseInitProperty(_ decl: Declaration) -> Bool { + guard decl.kind == .varInstance, + let parent = decl.parent, + parent.kind == .struct + else { return false } + + let implicitInits = parent.declarations.filter { $0.kind == .functionConstructor && $0.isImplicit } + + for implicitInit in implicitInits { + guard let initName = implicitInit.name, + let propertyName = decl.name + else { continue } + + let parameterNames = initName + .dropFirst("init(".count) + .dropLast(")".count) + .split(separator: ":") + .map(String.init) + + guard parameterNames.contains(propertyName) else { continue } + + // Case 1: Init referenced outside file -> properties must stay internal + if implicitInit.isReferencedOutsideFile(graph: graph) { + return true + } + + // Case 2: Init referenced in same file AND struct must remain internal + // (because it's used outside file or transitively exposed) + let initReferences = graph.references(to: implicitInit) + let hasAnyReference = !initReferences.isEmpty + if hasAnyReference { + if parent.isReferencedOutsideFile(graph: graph) { + return true + } + if isTransitivelyExposedOutsideFile(parent) { + return true + } + } + + // Case 3: Init referenced from a different type (or free function) in the same file. + // The properties need at least fileprivate access, so skip marking them as private. + let file = parent.location.file + let sameFileInitRefs = initReferences.filter { $0.location.file == file } + for ref in sameFileInitRefs { + guard let refParent = ref.parent else { continue } + guard let refContainingType = immediateContainingType(of: refParent) else { + // Called from a free function or top-level code + return true + } + + let parentLogicalType = logicalType(of: parent, inFile: file) + let refLogicalType = logicalType(of: refContainingType, inFile: file) + if parentLogicalType !== refLogicalType { + return true + } + } + + // Case 4: Init is referenced AND the parent struct is referenced by a #Preview macro. + // Preview macro expansions generate implicit types that cannot have access modifiers, + // so the properties must remain accessible. + if hasAnyReference, isReferencedByPreviewMacro(parent) { + return true + } + } + + return false + } + + /// Checks if a type is constrained by being used in the signature of another + /// internal declaration (in the same file) that must remain internal. + /// + /// In Swift, types used in a declaration's signature must be at least as accessible + /// as that declaration. If TypeA is used as a property type, return type, parameter + /// type, or generic constraint in TypeB/MemberB, and TypeB must remain internal + /// (because it's referenced outside the file or transitively exposed), then TypeA + /// cannot be made fileprivate/private. + /// + /// This complements isTransitivelyExposedOutsideFile() which handles cross-file + /// scenarios. This function handles same-file scenarios where the constraint chain + /// exists entirely within one file. + private func isConstrainedBySameFileTypeUsage(_ decl: Declaration) -> Bool { + let typeKinds: Set = [.enum, .struct, .class, .protocol] + guard typeKinds.contains(decl.kind) else { return false } + + var visited: Set = [] + return isConstrainedBySameFileTypeUsageRecursive(decl, visited: &visited) + } + + private func isConstrainedBySameFileTypeUsageRecursive( + _ decl: Declaration, + visited: inout Set + ) -> Bool { + let id = ObjectIdentifier(decl) + guard !visited.contains(id) else { return false } + + visited.insert(id) + + let typeKinds: Set = [.enum, .struct, .class, .protocol] + let file = decl.location.file + let refs = graph.references(to: decl) + + for ref in refs { + // Check if this reference is in ANY publicly exposable role + // (property type, return type, parameter type, generic constraint, etc.) + guard ref.role.isPubliclyExposable else { continue } + + // Must be in the same file (cross-file is handled by isTransitivelyExposedOutsideFile) + guard ref.location.file == file else { continue } + + // Get the declaration that uses this type in its signature + guard let usingDecl = ref.parent else { continue } + + // Find the containing type of that declaration + let containingType: Declaration? + if typeKinds.contains(usingDecl.kind) || usingDecl.kind.isExtensionKind { + // The using declaration IS a type (e.g., conformedType, inheritedType) + containingType = usingDecl + } else if let parent = usingDecl.parent, + typeKinds.contains(parent.kind) || parent.kind.isExtensionKind + { + // The using declaration is a member of a type + containingType = parent + } else { + continue + } + + guard let containingType else { continue } + + // Check if the containing type must remain internal + guard containingType.accessibility.value == .internal else { continue } + + // If the containing type is referenced outside the file, our type is constrained + if containingType.isReferencedOutsideFile(graph: graph) { + return true + } + + // If the containing type is transitively exposed outside the file + if isTransitivelyExposedOutsideFile(containingType) { + return true + } + + // Recursive: if the containing type is itself constrained + if isConstrainedBySameFileTypeUsageRecursive(containingType, visited: &visited) { + return true + } + } + + return false + } + + /// Checks if a declaration is referenced by a `#Preview` macro expansion. + /// + /// When `retainSwiftUIPreviews` is enabled, `#Preview` macros are retained. However, the + /// implicit declarations generated by these macros cannot have access modifiers. Types + /// referenced only by `#Preview` must remain `internal` — making them `fileprivate` or + /// `private` would break the preview because the macro expansion needs to access them. + private func isReferencedByPreviewMacro(_ decl: Declaration) -> Bool { + guard configuration.retainSwiftUIPreviews else { return false } + + let previewRegistryUsr = "s:21DeveloperToolsSupport15PreviewRegistryP" + + for ref in graph.references(to: decl) { + guard let parent = ref.parent, parent.isImplicit else { continue } + + // Check if this implicit parent has a related reference to PreviewRegistry, + // which identifies it as a #Preview macro expansion. + if parent.related.contains(where: { $0.usr == previewRegistryUsr }) { + return true + } + } + return false + } + + /// Determines the effective maximum accessibility a member can have based on its parent's accessibility. + /// + /// In Swift, a member's effective accessibility is constrained by its parent. This helper + /// ensures internal accessibility analysis respects these constraints when suggesting + /// more restrictive access levels. + private func effectiveMaximumAccessibility(for decl: Declaration) -> Accessibility? { + guard let parent = decl.parent else { return nil } + + let parentAccessibility = parent.accessibility.value + + switch parentAccessibility { + case .private: + return .private + case .fileprivate: + return .fileprivate + case .internal: + return .internal + case .public, .open: + return nil + } + } + + /// Checks if a declaration is referenced from a different type in the same file. + /// + /// For internal accessibility analysis, this determines whether to suggest `fileprivate` + /// versus `private` when a declaration is only used within its file. + /// + /// This uses the **immediate containing type** (not the top-level type) because: + /// - For nested types like `OuterStruct.InnerStruct`, a member of `InnerStruct` that's + /// accessed from code inside `OuterStruct` (but outside `InnerStruct`) needs `fileprivate` + /// - Using top-level type would incorrectly see both as belonging to `OuterStruct` + private func isReferencedFromDifferentTypeInSameFile(_ decl: Declaration) -> Bool { + let file = decl.location.file + let sameFileReferences = graph.references(to: decl).filter { $0.location.file == file } + + guard let declContainingType = immediateContainingType(of: decl) else { + return false + } + + let declLogicalType = logicalType(of: declContainingType, inFile: file) + + for ref in sameFileReferences { + guard let refParent = ref.parent else { continue } + guard let refContainingType = immediateContainingType(of: refParent) else { + // Reference from a free function or top-level code — no containing type. + // This is effectively a different type, so fileprivate is needed. + return true + } + + let refLogicalType = logicalType(of: refContainingType, inFile: file) + + if declLogicalType !== refLogicalType { + return true + } + } + return false + } + + /// Finds the immediate containing type of a declaration. + /// + /// For members (properties, methods, etc.), this returns their containing type. + /// For nested types, this returns the type that contains them (the outer type). + /// For top-level types, this returns the type itself (they are their own container). + private func immediateContainingType(of decl: Declaration) -> Declaration? { + let baseTypeKinds: Set = [.class, .struct, .enum, .protocol] + let typeKinds = baseTypeKinds.union(Declaration.Kind.extensionKinds) + + // For types, check if they have a parent type (nested type case). + // If so, return the parent type. If not (top-level), return the type itself. + if typeKinds.contains(decl.kind) { + if let parent = decl.parent, typeKinds.contains(parent.kind) { + return parent + } + return decl + } + + // Walk up the parent chain to find the first containing type + var current = decl.parent + while let parent = current { + if typeKinds.contains(parent.kind) { + return parent + } + current = parent.parent + } + + return nil + } + + /// Gets the logical type for comparison purposes when analyzing internal accessibility. + /// + /// For extensions of types in the SAME FILE, treats the extension as the extended type. + /// For extensions of types in DIFFERENT FILES, treats the extension as its own distinct type. + private func logicalType(of decl: Declaration, inFile file: SourceFile) -> Declaration? { + if decl.kind.isExtensionKind { + if let extendedDecl = try? graph.extendedDeclaration(forExtension: decl), + extendedDecl.location.file == file + { + return extendedDecl + } + return decl + } + return decl + } + + /// Checks if a type is transitively exposed outside its file through an API signature. + /// + /// A type is transitively exposed when it appears in the signature of a function, property, + /// or initializer (as return type, parameter type, etc.) where that API is referenced from + /// a different file. Even if the type itself is never directly referenced outside its file, + /// it must remain `internal` (not `fileprivate` or `private`) if it's part of an API that + /// is used from other files. + /// + /// For example, if `ScanProgress` is the return type of `runFullScanWithStreaming()`, and + /// that function is called from another file, then `ScanProgress` is transitively exposed + /// and should not be marked as redundantly internal. + /// + /// This check is recursive: if TypeA is used as a property type in Container, and Container + /// is used as a property type in OuterContainer, and OuterContainer is referenced from + /// outside the file, then TypeA is transitively exposed through the chain. + private func isTransitivelyExposedOutsideFile(_ decl: Declaration) -> Bool { + var visited: Set = [] + return isTransitivelyExposedOutsideFileRecursive(decl, visited: &visited) + } + + private func isTransitivelyExposedOutsideFileRecursive(_ decl: Declaration, visited: inout Set) -> Bool { + let id = ObjectIdentifier(decl) + guard !visited.contains(id) else { return false } + + visited.insert(id) + + let refs = graph.references(to: decl) + + for ref in refs { + // Check if this reference is in an API signature role (return type, parameter type, etc.) + guard ref.role.isPubliclyExposable else { continue } + + // Get the parent declaration (the function/property that uses this type in its signature) + guard let parent = ref.parent else { continue } + + // Check if that parent API is referenced from outside this file + if parent.isReferencedOutsideFile(graph: graph) { + return true + } + + // For properties, also check if they could be accessed from outside the file + // through their containing type. The property's type is exposed when: + // 1. The property is internal (accessible from outside the file) + // 2. The containing type is used from outside the file OR transitively exposed + // 3. The property is not actually referenced from outside (already checked above) + if parent.kind.isVariableKind, + parent.accessibility.value == .internal || parent.accessibility.isAccessibleCrossModule + { + if let containingType = parent.parent { + // Direct reference from outside the file + if containingType.isReferencedOutsideFile(graph: graph) { + return true + } + // Recursive: the containing type itself is transitively exposed + if isTransitivelyExposedOutsideFileRecursive(containingType, visited: &visited) { + return true + } + } + } + } + + return false + } + + /// Checks if a type is transitively exposed from a different type within the same file. + /// + /// This is similar to `isTransitivelyExposedOutsideFile`, but checks for exposure within + /// the same file from a different type. When a type is used as a return type or parameter + /// type of a function that is called from a different type in the same file, the type + /// needs to be at least `fileprivate` (not `private`). + /// + /// For example: + /// ```swift + /// class ClassA { + /// enum Status { case active } // Only directly referenced in ClassA + /// func getStatus() -> Status { .active } // Called from ClassB + /// } + /// class ClassB { + /// func use() { _ = ClassA().getStatus() } // Uses Status transitively + /// } + /// ``` + /// Here, `Status` should be suggested as `fileprivate`, not `private`. + /// + /// This check is recursive: if TypeA is used as a property type in Container, and Container + /// is used as a property type in another type that is accessed from a different type in + /// the same file, then TypeA needs fileprivate. + private func isTransitivelyExposedFromDifferentTypeInSameFile(_ decl: Declaration) -> Bool { + let file = decl.location.file + + guard let declContainingType = immediateContainingType(of: decl) else { + return false + } + + let declLogicalType = logicalType(of: declContainingType, inFile: file) + var visited: Set = [] + + return isTransitivelyExposedFromDifferentTypeInSameFileRecursive( + decl, + declLogicalType: declLogicalType, + file: file, + visited: &visited + ) + } + + private func isTransitivelyExposedFromDifferentTypeInSameFileRecursive( + _ decl: Declaration, + declLogicalType: Declaration?, + file: SourceFile, + visited: inout Set + ) -> Bool { + let id = ObjectIdentifier(decl) + guard !visited.contains(id) else { return false } + + visited.insert(id) + + let refs = graph.references(to: decl) + + for ref in refs { + // Check if this reference is in an API signature role (return type, parameter type, etc.) + guard ref.role.isPubliclyExposable else { continue } + + // Get the parent declaration (the function/property that uses this type in its signature) + guard let parent = ref.parent else { continue } + + // Check references to that parent from the same file + let parentRefs = graph.references(to: parent).filter { $0.location.file == file } + + for parentRef in parentRefs { + guard let refParent = parentRef.parent else { continue } + guard let refContainingType = immediateContainingType(of: refParent) else { + // Reference from a free function or top-level code — no containing type. + return true + } + + let refLogicalType = logicalType(of: refContainingType, inFile: file) + + if declLogicalType !== refLogicalType { + return true + } + } + + // For properties, also check if the containing type is transitively exposed + // from a different type in the same file + if parent.kind.isVariableKind, + parent.accessibility.value == .internal || parent.accessibility.isAccessibleCrossModule + { + if let containingType = parent.parent { + if isTransitivelyExposedFromDifferentTypeInSameFileRecursive( + containingType, + declLogicalType: declLogicalType, + file: file, + visited: &visited + ) { + return true + } + } + } + } + + return false + } + + /// Checks if a type is used in a protocol requirement's signature (return type, parameter type, etc.). + /// + /// When a type is used as the return type or parameter type of a protocol requirement method, + /// the type must be at least `fileprivate` - making it `private` would cause a compiler error + /// because the protocol method's signature would expose an inaccessible type. + /// + /// For example, in NSViewRepresentable: + /// ```swift + /// class FocusableNSView: NSView { ... } // Used as return type of makeNSView + /// + /// struct FocusClaimingView: NSViewRepresentable { + /// func makeNSView(context: Context) -> FocusableNSView { ... } + /// } + /// ``` + /// Here, `FocusableNSView` cannot be `private` because it's exposed through `makeNSView`'s signature. + private func isUsedInProtocolRequirementSignature(_ decl: Declaration) -> Bool { + let refs = graph.references(to: decl) + + for ref in refs { + // Check if this reference is in an API signature role (return type, parameter type, etc.) + guard ref.role.isPubliclyExposable else { continue } + + // Get the parent declaration (the function/property that uses this type in its signature) + guard let parent = ref.parent else { continue } + + // Check if that parent is a protocol requirement + if isProtocolRequirement(parent) { + return true + } + } + + return false + } + + /// Checks if a type is used in the signature of a method that conforms to an external protocol. + /// + /// Types used in external protocol requirement signatures must remain `internal` because: + /// 1. The method implementing the protocol requirement can't be more restrictive than the protocol + /// 2. The types used in its signature must be at least as accessible as the method + /// + /// For example, with NSViewRepresentable: + /// ```swift + /// class FocusableNSView: NSView { ... } // Must stay internal! + /// + /// struct FocusClaimingView: NSViewRepresentable { + /// func makeNSView(context: Context) -> FocusableNSView { ... } // Protocol requirement + /// func updateNSView(_ nsView: FocusableNSView, context: Context) { ... } // Protocol requirement + /// } + /// ``` + /// Here, `FocusableNSView` cannot be made `fileprivate` or `private` because the protocol + /// methods that use it must remain at the protocol's required accessibility level. + private func isUsedInExternalProtocolRequirementSignature(_ decl: Declaration) -> Bool { + let refs = graph.references(to: decl) + + for ref in refs { + // Check if this reference is in an API signature role (return type, parameter type, etc.) + guard ref.role.isPubliclyExposable else { continue } + + // Get the parent declaration (the function/property that uses this type in its signature) + guard let parent = ref.parent else { continue } + + // Check if that parent conforms to an external protocol requirement + if isExternalProtocolRequirement(parent) { + return true + } + } + + return false + } + + /// Checks if a declaration implements an external protocol requirement. + /// + /// External protocols are those defined outside our codebase (e.g., NSViewRepresentable, + /// Codable, etc.). When a declaration implements such a protocol, its accessibility is + /// constrained by the protocol. + private func isExternalProtocolRequirement(_ decl: Declaration) -> Bool { + // Check for .related references FROM this declaration to protocol members + // where the protocol is external (not in our graph). + for ref in decl.related where ref.declarationKind.isProtocolMemberConformingKind { + // If we can't find the declaration in our graph, it's external + if graph.declaration(withUsr: ref.usr) == nil, ref.name == decl.name { + return true + } + } + + return false + } +} diff --git a/Sources/SourceGraph/SourceGraph.swift b/Sources/SourceGraph/SourceGraph.swift index 661e66664..08e78ab5d 100644 --- a/Sources/SourceGraph/SourceGraph.swift +++ b/Sources/SourceGraph/SourceGraph.swift @@ -9,6 +9,8 @@ public final class SourceGraph { public private(set) var redundantProtocols: [Declaration: (references: Set, inherited: Set)] = [:] public private(set) var rootDeclarations: Set = [] public private(set) var redundantPublicAccessibility: [Declaration: Set] = [:] + public private(set) var redundantInternalAccessibility: [Declaration: (files: Set, suggestedAccessibility: Accessibility?)] = [:] + public private(set) var redundantFilePrivateAccessibility: [Declaration: (files: Set, containingTypeName: String?)] = [:] public private(set) var rootReferences: Set = [] public private(set) var allReferences: Set = [] public private(set) var retainedDeclarations: Set = [] @@ -87,6 +89,25 @@ public final class SourceGraph { _ = redundantPublicAccessibility.removeValue(forKey: declaration) } + func markRedundantInternalAccessibility(_ declaration: Declaration, file: SourceFile, suggestedAccessibility: Accessibility?) { + if let existing = redundantInternalAccessibility[declaration] { + var files = existing.files + files.insert(file) + redundantInternalAccessibility[declaration] = (files: files, suggestedAccessibility: existing.suggestedAccessibility) + } else { + redundantInternalAccessibility[declaration] = (files: [file], suggestedAccessibility: suggestedAccessibility) + } + } + + func markRedundantFilePrivateAccessibility(_ declaration: Declaration, file: SourceFile, containingTypeName: String?) { + if var existing = redundantFilePrivateAccessibility[declaration] { + existing.files.insert(file) + redundantFilePrivateAccessibility[declaration] = existing + } else { + redundantFilePrivateAccessibility[declaration] = (files: [file], containingTypeName: containingTypeName) + } + } + func markIgnored(_ declaration: Declaration) { _ = ignoredDeclarations.insert(declaration) } @@ -298,7 +319,7 @@ public final class SourceGraph { inheritedTypeReferences(of: decl).compactMap { declaration(withUsr: $0.usr) } } - func immediateSubclasses(of decl: Declaration) -> Set { + private func immediateSubclasses(of decl: Declaration) -> Set { references(to: decl) .filter { $0.kind == .related && $0.declarationKind == .class } .flatMap { $0.parent?.usrs ?? [] } diff --git a/Sources/SourceGraph/SourceGraphDebugger.swift b/Sources/SourceGraph/SourceGraphDebugger.swift index e62f6ddba..10c78d958 100644 --- a/Sources/SourceGraph/SourceGraphDebugger.swift +++ b/Sources/SourceGraph/SourceGraphDebugger.swift @@ -14,7 +14,7 @@ final class SourceGraphDebugger { // MARK: - Private - func describe(_ declarations: [Declaration]) { + private func describe(_ declarations: [Declaration]) { for (index, declaration) in declarations.enumerated() { describe(declaration) diff --git a/Sources/SourceGraph/SourceGraphMutatorRunner.swift b/Sources/SourceGraph/SourceGraphMutatorRunner.swift index c9fc12919..3193ee154 100644 --- a/Sources/SourceGraph/SourceGraphMutatorRunner.swift +++ b/Sources/SourceGraph/SourceGraphMutatorRunner.swift @@ -17,6 +17,10 @@ public final class SourceGraphMutatorRunner { // Must come before ProtocolExtensionReferenceBuilder because it removes references // from the extension to the protocol, thus making them appear to be unknown. ExtensionReferenceBuilder.self, + // Must come after ExtensionReferenceBuilder so that it can detect redundant accessibility + // on properties that are used in extensions. + RedundantInternalAccessibilityMarker.self, + RedundantFilePrivateAccessibilityMarker.self, // Must come before ProtocolConformanceReferenceBuilder because it removes references to // conformed protocols, which CodingKeyEnumReferenceBuilder needs to inspect before removal. // It must also come after ExtensionReferenceBuilder as some types may declare conformance diff --git a/Sources/SyntaxAnalysis/CommentCommand.swift b/Sources/SyntaxAnalysis/CommentCommand.swift index 7379a763d..8943dae7d 100644 --- a/Sources/SyntaxAnalysis/CommentCommand.swift +++ b/Sources/SyntaxAnalysis/CommentCommand.swift @@ -29,7 +29,7 @@ extension CommentCommand { } ?? [] } - static func parse(_ rawCommand: String) -> Self? { + private static func parse(_ rawCommand: String) -> Self? { if rawCommand == "ignore" { return .ignore } else if rawCommand == "ignore:all" { diff --git a/Sources/SyntaxAnalysis/DeclarationSyntaxVisitor.swift b/Sources/SyntaxAnalysis/DeclarationSyntaxVisitor.swift index 985dad60a..ce12a7adb 100644 --- a/Sources/SyntaxAnalysis/DeclarationSyntaxVisitor.swift +++ b/Sources/SyntaxAnalysis/DeclarationSyntaxVisitor.swift @@ -222,7 +222,7 @@ public final class DeclarationSyntaxVisitor: PeripherySyntaxVisitor { } } - func visitVariableTupleBinding(node: VariableDeclSyntax, pattern: TuplePatternSyntax, typeTuple: TupleTypeElementListSyntax?, initializerTuple: LabeledExprListSyntax?) { + private func visitVariableTupleBinding(node: VariableDeclSyntax, pattern: TuplePatternSyntax, typeTuple: TupleTypeElementListSyntax?, initializerTuple: LabeledExprListSyntax?) { let elements = Array(pattern.elements) let types: [TupleTypeElementSyntax?] = typeTuple?.map(\.self) ?? Array(repeating: nil, count: elements.count) let initializers: [LabeledExprSyntax?] = initializerTuple?.map(\.self) ?? Array(repeating: nil, count: elements.count) diff --git a/Sources/SyntaxAnalysis/MultiplexingSyntaxVisitor.swift b/Sources/SyntaxAnalysis/MultiplexingSyntaxVisitor.swift index 9222f86c0..5de68831c 100644 --- a/Sources/SyntaxAnalysis/MultiplexingSyntaxVisitor.swift +++ b/Sources/SyntaxAnalysis/MultiplexingSyntaxVisitor.swift @@ -95,8 +95,8 @@ public final class MultiplexingSyntaxVisitor: SyntaxVisitor { public let sourceFile: SourceFile public let syntax: SourceFileSyntax public let locationConverter: SourceLocationConverter - let sourceLocationBuilder: SourceLocationBuilder - let swiftVersion: SwiftVersion + private let sourceLocationBuilder: SourceLocationBuilder + private let swiftVersion: SwiftVersion private var visitors: [PeripherySyntaxVisitor] = [] diff --git a/Sources/SyntaxAnalysis/TypeSyntaxInspector.swift b/Sources/SyntaxAnalysis/TypeSyntaxInspector.swift index 370f60970..2403963a4 100644 --- a/Sources/SyntaxAnalysis/TypeSyntaxInspector.swift +++ b/Sources/SyntaxAnalysis/TypeSyntaxInspector.swift @@ -28,7 +28,7 @@ struct TypeSyntaxInspector { // MARK: - Private - func types(for typeSyntax: TypeSyntax) -> Set { + private func types(for typeSyntax: TypeSyntax) -> Set { if let identifierType = typeSyntax.as(IdentifierTypeSyntax.self) { // Simple type. var result: Set = identifierType.genericArgumentClause?.arguments.flatMapSet { diff --git a/Sources/SyntaxAnalysis/UnusedParameterParser.swift b/Sources/SyntaxAnalysis/UnusedParameterParser.swift index cf7250f4f..d6b7ec947 100644 --- a/Sources/SyntaxAnalysis/UnusedParameterParser.swift +++ b/Sources/SyntaxAnalysis/UnusedParameterParser.swift @@ -131,7 +131,7 @@ struct UnusedParameterParser { self.parseProtocols = parseProtocols } - func parse() -> [Function] { + private func parse() -> [Function] { parse(node: syntax, collecting: Function.self) } diff --git a/Sources/XcodeSupport/XcodeTarget.swift b/Sources/XcodeSupport/XcodeTarget.swift index bb184b94f..89eb06d68 100644 --- a/Sources/XcodeSupport/XcodeTarget.swift +++ b/Sources/XcodeSupport/XcodeTarget.swift @@ -4,7 +4,7 @@ import SystemPackage import XcodeProj public final class XcodeTarget { - let project: XcodeProject + private let project: XcodeProject private let target: PBXTarget private var files: [ProjectFileKind: Set] = [:] diff --git a/Tests/AccessibilityTests/AccessibilityProject/Sources/MainTarget/main.swift b/Tests/AccessibilityTests/AccessibilityProject/Sources/MainTarget/main.swift index 534fe6931..00e4b6e49 100644 --- a/Tests/AccessibilityTests/AccessibilityProject/Sources/MainTarget/main.swift +++ b/Tests/AccessibilityTests/AccessibilityProject/Sources/MainTarget/main.swift @@ -5,6 +5,8 @@ PublicDeclarationInInternalParentRetainer().retain() PublicExtensionOnRedundantPublicKindRetainer().retain() IgnoreCommentCommandRetainer().retain() IgnoreAllCommentCommandRetainer().retain() +RedundantFilePrivateClassRetainer().retain() +InternalPropertyUsedInExtensionRetainer().retain() _ = PublicTypeUsedAsPublicInitializerParameterTypeRetainer() @@ -79,3 +81,17 @@ inlinableFunction() // Associated types _ = PublicInheritedAssociatedTypeClass().items _ = PublicInheritedAssociatedTypeDefaultTypeClass().items + +// Redundant accessibility tests +TrulyRedundantFilePrivateMembersRetainer().retain() +ProtocolRequirementAccessibilityRetainer().retain() +PropertyWrapperAccessibilityRetainer().retain() +NestedTypeAccessibilityRetainer().retain() +InternalSuggestingPrivateVsFileprivateRetainer().retain() +ImplicitlyInternalRetainer().retain() +NotRedundantInternalClassComponents_Support().useImplicitlyInternalStruct() +InternalTypeAsReturnTypeRetainer().retain() +InternalTypeTransitivelyExposedInSameFileRetainer().retain() +TransitiveAccessExposureRetainer().retain() +MethodCalledFromFreeFunctionRetainer().retain() +MemberwiseInitCalledFromDifferentTypeRetainer().retain() diff --git a/Tests/AccessibilityTests/AccessibilityProject/Sources/TargetA/EnumCaseAccessibility.swift b/Tests/AccessibilityTests/AccessibilityProject/Sources/TargetA/EnumCaseAccessibility.swift new file mode 100644 index 000000000..588d71a10 --- /dev/null +++ b/Tests/AccessibilityTests/AccessibilityProject/Sources/TargetA/EnumCaseAccessibility.swift @@ -0,0 +1,32 @@ +/* + EnumCaseAccessibility.swift + Tests that enum cases are NOT flagged as redundant internal. + + Enum cases cannot have explicit access modifiers in Swift - they inherit + the accessibility of their containing enum. Suggesting to make them private + or fileprivate would cause a syntax error. +*/ + +// Internal enum with cases only used in this file - should NOT flag the cases. +internal enum InternalEnumWithCasesUsedOnlyInSameFile { + case usedCase + case anotherUsedCase +} + +// Public enum with internal cases - cases should NOT be flagged. +public enum PublicEnumWithInternalCases { + case internalCase + case anotherInternalCase +} + +// Usage within the file to exercise the enum cases. +public class EnumCaseAccessibilityRetainer { + public init() {} + + public func retain() { + _ = InternalEnumWithCasesUsedOnlyInSameFile.usedCase + _ = InternalEnumWithCasesUsedOnlyInSameFile.anotherUsedCase + _ = PublicEnumWithInternalCases.internalCase + _ = PublicEnumWithInternalCases.anotherInternalCase + } +} diff --git a/Tests/AccessibilityTests/AccessibilityProject/Sources/TargetA/ExternalProtocolConformanceAccessibility.swift b/Tests/AccessibilityTests/AccessibilityProject/Sources/TargetA/ExternalProtocolConformanceAccessibility.swift new file mode 100644 index 000000000..77458b0db --- /dev/null +++ b/Tests/AccessibilityTests/AccessibilityProject/Sources/TargetA/ExternalProtocolConformanceAccessibility.swift @@ -0,0 +1,27 @@ +/* + ExternalProtocolConformanceAccessibility.swift + Tests that types conforming to external protocols are NOT flagged as redundant internal. +*/ + +import ExternalTarget + +// Internal struct conforming to external protocol - should NOT be flagged. +internal struct InternalStructConformingToExternalProtocol: ExternalProtocol { + func someExternalProtocolMethod() {} +} + +// Implicitly internal class conforming to external protocol - should NOT be flagged. +class ImplicitlyInternalClassConformingToExternalProtocol: ExternalProtocol { + func someExternalProtocolMethod() {} +} + +// Used to ensure these types are referenced. +public class ExternalProtocolConformanceRetainer { + public init() {} + public func retain() { + let s = InternalStructConformingToExternalProtocol() + s.someExternalProtocolMethod() + let c = ImplicitlyInternalClassConformingToExternalProtocol() + c.someExternalProtocolMethod() + } +} diff --git a/Tests/AccessibilityTests/AccessibilityProject/Sources/TargetA/ExternalProtocolSignatureType.swift b/Tests/AccessibilityTests/AccessibilityProject/Sources/TargetA/ExternalProtocolSignatureType.swift new file mode 100644 index 000000000..f6214d34d --- /dev/null +++ b/Tests/AccessibilityTests/AccessibilityProject/Sources/TargetA/ExternalProtocolSignatureType.swift @@ -0,0 +1,38 @@ +/* + ExternalProtocolSignatureType.swift + Tests that types used in external protocol requirement signatures are NOT + flagged as redundant internal. + + When a type is used as the return type or parameter type of a method that + implements an external protocol requirement (like NSViewRepresentable.makeNSView), + the type must remain internal because the protocol method can't be more + restrictive than the protocol requires. + + This mimics patterns like: + - NSViewRepresentable/UIViewRepresentable returning custom NSView/UIView subclasses + - Codable types with custom coding containers +*/ + +import Foundation + +// This type is used as the return type of an external protocol requirement. +// It should NOT be flagged as redundant internal because Equatable.== must +// remain internal, and its parameter types must be at least as accessible. +internal struct TypeUsedInExternalProtocolSignature: Equatable { + internal var value: Int + + // The == function is an external protocol requirement (from Equatable). + // Since this struct conforms to Equatable, the == method's parameter type + // (this struct) must remain internal. +} + +// Usage to retain the type +public class ExternalProtocolSignatureTypeRetainer { + public init() {} + + public func retain() { + let a = TypeUsedInExternalProtocolSignature(value: 1) + let b = TypeUsedInExternalProtocolSignature(value: 2) + _ = a == b + } +} diff --git a/Tests/AccessibilityTests/AccessibilityProject/Sources/TargetA/InternalPropertyOwner.swift b/Tests/AccessibilityTests/AccessibilityProject/Sources/TargetA/InternalPropertyOwner.swift new file mode 100644 index 000000000..c4969afc8 --- /dev/null +++ b/Tests/AccessibilityTests/AccessibilityProject/Sources/TargetA/InternalPropertyOwner.swift @@ -0,0 +1,4 @@ +// InternalPropertyOwner.swift +internal class InternalPropertyOwner { + internal var value: Int = 42 +} \ No newline at end of file diff --git a/Tests/AccessibilityTests/AccessibilityProject/Sources/TargetA/InternalPropertyOwner_Extension.swift b/Tests/AccessibilityTests/AccessibilityProject/Sources/TargetA/InternalPropertyOwner_Extension.swift new file mode 100644 index 000000000..aa70c5878 --- /dev/null +++ b/Tests/AccessibilityTests/AccessibilityProject/Sources/TargetA/InternalPropertyOwner_Extension.swift @@ -0,0 +1,6 @@ +// InternalPropertyUserExtension.swift +extension InternalPropertyOwner { + func printValue() { + print(value) + } +} \ No newline at end of file diff --git a/Tests/AccessibilityTests/AccessibilityProject/Sources/TargetA/InternalPropertyUsedInExtension.swift b/Tests/AccessibilityTests/AccessibilityProject/Sources/TargetA/InternalPropertyUsedInExtension.swift new file mode 100644 index 000000000..25dc53e71 --- /dev/null +++ b/Tests/AccessibilityTests/AccessibilityProject/Sources/TargetA/InternalPropertyUsedInExtension.swift @@ -0,0 +1,20 @@ +// InternalPropertyUsedInExtension.swift +// Tests the case where an internal property is used in an extension in a different file +// This should NOT be flagged as redundant + +internal class InternalPropertyUsedInExtension { + internal var propertyUsedInExtension: String = "test" + internal var propertyOnlyUsedInSameFile: String = "test" // This should be flagged as redundant + + func useSameFileProperty() { + print(propertyOnlyUsedInSameFile) + } +} + +public class InternalPropertyUsedInExtensionRetainer { + public init() {} + public func retain() { + let instance = InternalPropertyUsedInExtension() + instance.useSameFileProperty() + } +} diff --git a/Tests/AccessibilityTests/AccessibilityProject/Sources/TargetA/InternalPropertyUsedInExtension_Extension.swift b/Tests/AccessibilityTests/AccessibilityProject/Sources/TargetA/InternalPropertyUsedInExtension_Extension.swift new file mode 100644 index 000000000..48f8bba34 --- /dev/null +++ b/Tests/AccessibilityTests/AccessibilityProject/Sources/TargetA/InternalPropertyUsedInExtension_Extension.swift @@ -0,0 +1,9 @@ +// InternalPropertyExtension.swift +// Extension that uses the internal property from InternalPropertyUsedInExtension +// This should prevent the property from being flagged as redundant + +extension InternalPropertyUsedInExtension { + func usePropertyInExtension() { + print(propertyUsedInExtension) // This reference should prevent propertyUsedInExtension from being flagged as redundant + } +} diff --git a/Tests/AccessibilityTests/AccessibilityProject/Sources/TargetA/InternalSuggestingPrivateVsFileprivate.swift b/Tests/AccessibilityTests/AccessibilityProject/Sources/TargetA/InternalSuggestingPrivateVsFileprivate.swift new file mode 100644 index 000000000..5e01a29d2 --- /dev/null +++ b/Tests/AccessibilityTests/AccessibilityProject/Sources/TargetA/InternalSuggestingPrivateVsFileprivate.swift @@ -0,0 +1,46 @@ +/* + InternalSuggestingPrivateVsFileprivate.swift + Tests that internal declarations correctly suggest private vs fileprivate + based on whether they're accessed from different types in the same file. +*/ + +// Class with internal property only used within its own type - should suggest private. +public class ClassWithInternalPropertySuggestingPrivate { + // Should be flagged as redundant internal (can be 'private'). + internal var usedOnlyInOwnType: Int = 0 + + public init() {} + + public func useProperty() { + _ = usedOnlyInOwnType + } +} + +// Two classes where one accesses the other's internal property - should suggest fileprivate. +public class ClassA { + // Should be flagged as redundant internal (can be 'fileprivate'). + internal var sharedWithClassB: String = "" + + public init() {} +} + +public class ClassB { + public init() {} + + public func accessClassA(_ a: ClassA) { + _ = a.sharedWithClassB + } +} + +// Used by main.swift to ensure these are referenced. +public class InternalSuggestingPrivateVsFileprivateRetainer { + public init() {} + public func retain() { + let obj1 = ClassWithInternalPropertySuggestingPrivate() + obj1.useProperty() + + let a = ClassA() + let b = ClassB() + b.accessClassA(a) + } +} diff --git a/Tests/AccessibilityTests/AccessibilityProject/Sources/TargetA/InternalTestableImportUsage.swift b/Tests/AccessibilityTests/AccessibilityProject/Sources/TargetA/InternalTestableImportUsage.swift new file mode 100644 index 000000000..a8d44bd3e --- /dev/null +++ b/Tests/AccessibilityTests/AccessibilityProject/Sources/TargetA/InternalTestableImportUsage.swift @@ -0,0 +1,11 @@ +import Foundation + +// Should NOT be flagged as redundant - used from test via @testable import +internal class InternalUsedOnlyInTest { + internal func testOnlyMethod() {} +} + +// Should NOT be flagged - used from both test AND production code +internal class InternalUsedInBoth { + internal func sharedMethod() {} +} diff --git a/Tests/AccessibilityTests/AccessibilityProject/Sources/TargetA/InternalTestableImportUsage_Support.swift b/Tests/AccessibilityTests/AccessibilityProject/Sources/TargetA/InternalTestableImportUsage_Support.swift new file mode 100644 index 000000000..e6c6cc4e8 --- /dev/null +++ b/Tests/AccessibilityTests/AccessibilityProject/Sources/TargetA/InternalTestableImportUsage_Support.swift @@ -0,0 +1,10 @@ +import Foundation + +// This file uses InternalUsedInBoth from production code within the same module +// to ensure it's not flagged as redundant internal (since it's used both in production and tests) + +struct InternalTestableImportRetainer { + func retain() { + _ = InternalUsedInBoth().sharedMethod() + } +} diff --git a/Tests/AccessibilityTests/AccessibilityProject/Sources/TargetA/InternalTypeAsReturnType.swift b/Tests/AccessibilityTests/AccessibilityProject/Sources/TargetA/InternalTypeAsReturnType.swift new file mode 100644 index 000000000..608c2f6e7 --- /dev/null +++ b/Tests/AccessibilityTests/AccessibilityProject/Sources/TargetA/InternalTypeAsReturnType.swift @@ -0,0 +1,25 @@ +// InternalTypeAsReturnType.swift +// Tests for internal types used as return types of functions called from other files. +// These should NOT be flagged as redundant internal. + +// This internal enum is used as the return type of a function that is called from another file. +// Even though InternalReturnTypeEnum is never directly referenced outside this file, +// it is transitively exposed through the function's return type. +internal enum InternalReturnTypeEnum { + case value +} + +internal class InternalReturnTypeContainer { + func getEnum() -> InternalReturnTypeEnum { + return .value + } +} + +public class InternalTypeAsReturnTypeRetainer { + public init() {} + + public func retain() { + let container = InternalReturnTypeContainer() + _ = container.getEnum() + } +} diff --git a/Tests/AccessibilityTests/AccessibilityProject/Sources/TargetA/InternalTypeAsReturnType_Consumer.swift b/Tests/AccessibilityTests/AccessibilityProject/Sources/TargetA/InternalTypeAsReturnType_Consumer.swift new file mode 100644 index 000000000..728aebddf --- /dev/null +++ b/Tests/AccessibilityTests/AccessibilityProject/Sources/TargetA/InternalTypeAsReturnType_Consumer.swift @@ -0,0 +1,11 @@ +// InternalTypeAsReturnType_Consumer.swift +// Consumer that calls the function with internal return type from a different file. +// This creates the transitive exposure of InternalReturnTypeEnum. + +class InternalReturnTypeConsumer { + func consume() { + let container = InternalReturnTypeContainer() + // This call uses InternalReturnTypeEnum transitively through the return type + let _ = container.getEnum() + } +} diff --git a/Tests/AccessibilityTests/AccessibilityProject/Sources/TargetA/InternalTypeTransitivelyExposedInSameFile.swift b/Tests/AccessibilityTests/AccessibilityProject/Sources/TargetA/InternalTypeTransitivelyExposedInSameFile.swift new file mode 100644 index 000000000..0f7c1d645 --- /dev/null +++ b/Tests/AccessibilityTests/AccessibilityProject/Sources/TargetA/InternalTypeTransitivelyExposedInSameFile.swift @@ -0,0 +1,37 @@ +// InternalTypeTransitivelyExposedInSameFile.swift +// Tests for internal types that are transitively exposed within the same file +// but from a different type. These should suggest fileprivate, not private. + +class TransitiveExposureClassA { + // This enum is only directly referenced within ClassA, but it's the return type + // of getStatus() which IS called from ClassB in the same file. + // It should be suggested as fileprivate (not private) because ClassB uses it transitively. + internal enum TransitivelyExposedStatus { + case active + case inactive + } + + func getStatus() -> TransitivelyExposedStatus { + .active + } +} + +class TransitiveExposureClassB { + func checkStatus() { + let a = TransitiveExposureClassA() + // This call uses TransitivelyExposedStatus transitively through the return type + let _ = a.getStatus() + } +} + +// Retainer that only uses ClassB (not ClassA.getStatus() directly) +// This ensures getStatus() is only referenced from within this file +public class InternalTypeTransitivelyExposedInSameFileRetainer { + public init() {} + + public func retain() { + // Only call checkStatus(), not getStatus() directly + // So getStatus() is only referenced from checkStatus() in this same file + TransitiveExposureClassB().checkStatus() + } +} diff --git a/Tests/AccessibilityTests/AccessibilityProject/Sources/TargetA/MemberwiseInitCalledFromDifferentType.swift b/Tests/AccessibilityTests/AccessibilityProject/Sources/TargetA/MemberwiseInitCalledFromDifferentType.swift new file mode 100644 index 000000000..bf3d6af88 --- /dev/null +++ b/Tests/AccessibilityTests/AccessibilityProject/Sources/TargetA/MemberwiseInitCalledFromDifferentType.swift @@ -0,0 +1,27 @@ +/* + MemberwiseInitCalledFromDifferentType.swift + Tests that struct properties are NOT flagged as redundant internal when the + struct's implicit memberwise initializer is called from a different type + in the same file. + + When a ViewModifier or another type creates a struct using its memberwise init, + the properties need at least fileprivate access and should not be flagged. +*/ + +struct MemberwiseInitStruct { + var crossTypeProperty1: String + var crossTypeProperty2: Int +} + +class MemberwiseInitCaller { + func create() -> MemberwiseInitStruct { + MemberwiseInitStruct(crossTypeProperty1: "hello", crossTypeProperty2: 42) + } +} + +public class MemberwiseInitCalledFromDifferentTypeRetainer { + public init() {} + public func retain() { + _ = MemberwiseInitCaller().create() + } +} diff --git a/Tests/AccessibilityTests/AccessibilityProject/Sources/TargetA/MethodCalledFromFreeFunction.swift b/Tests/AccessibilityTests/AccessibilityProject/Sources/TargetA/MethodCalledFromFreeFunction.swift new file mode 100644 index 000000000..0244c1fb4 --- /dev/null +++ b/Tests/AccessibilityTests/AccessibilityProject/Sources/TargetA/MethodCalledFromFreeFunction.swift @@ -0,0 +1,29 @@ +/* + MethodCalledFromFreeFunction.swift + Tests that type members are NOT flagged as private when they are called + from a free function in the same file. + + A free function has no containing type, so accessing a type's member from + a free function requires at least fileprivate access, not private. +*/ + +public class ClassWithMethodCalledFromFreeFunction { + public init() {} + + func methodCalledFromFreeFunction() -> String { "result" } + var propertyUsedFromFreeFunction: Int = 0 +} + +func freeFunctionCallingMethod() { + let obj = ClassWithMethodCalledFromFreeFunction() + _ = obj.methodCalledFromFreeFunction() + _ = obj.propertyUsedFromFreeFunction +} + +// Retainer to ensure the free function is used +public class MethodCalledFromFreeFunctionRetainer { + public init() {} + public func retain() { + freeFunctionCallingMethod() + } +} diff --git a/Tests/AccessibilityTests/AccessibilityProject/Sources/TargetA/NestedTypeAccessibility.swift b/Tests/AccessibilityTests/AccessibilityProject/Sources/TargetA/NestedTypeAccessibility.swift new file mode 100644 index 000000000..a5e185759 --- /dev/null +++ b/Tests/AccessibilityTests/AccessibilityProject/Sources/TargetA/NestedTypeAccessibility.swift @@ -0,0 +1,43 @@ +/* + NestedTypeAccessibility.swift + Tests nested type accessibility analysis and redundancy suppression. +*/ + +/* Public class with internal nested types that are only used within the file. */ +public class OuterClassWithNestedTypes { + /* Should be flagged as redundant (not used outside file). */ + internal struct NestedStruct { + /* Should NOT be flagged if parent is already flagged (nested redundancy suppression). */ + internal var nestedProperty: Int = 0 + + /* Should NOT be flagged if parent is already flagged (nested redundancy suppression). */ + internal func nestedMethod() {} + } + + /* Should be flagged as redundant (not used outside file). */ + internal class NestedClass { + /* Should NOT be flagged if parent is already flagged (nested redundancy suppression). */ + internal var anotherProperty: String = "" + } + + internal var nested: NestedStruct = .init() + + public init() {} + + public func useNested() { + _ = nested.nestedProperty + nested.nestedMethod() + + let nc = NestedClass() + _ = nc.anotherProperty + } +} + +/* Used by main.swift to ensure these are referenced. */ +public class NestedTypeAccessibilityRetainer { + public init() {} + public func retain() { + let obj = OuterClassWithNestedTypes() + obj.useNested() + } +} diff --git a/Tests/AccessibilityTests/AccessibilityProject/Sources/TargetA/NotRedundantFilePrivateClass.swift b/Tests/AccessibilityTests/AccessibilityProject/Sources/TargetA/NotRedundantFilePrivateClass.swift new file mode 100644 index 000000000..aabe2452b --- /dev/null +++ b/Tests/AccessibilityTests/AccessibilityProject/Sources/TargetA/NotRedundantFilePrivateClass.swift @@ -0,0 +1,23 @@ +/* + NotRedundantFilePrivateClass.swift + Tests that a fileprivate class is NOT flagged as redundant when it's used from a different type in the same file +*/ + +fileprivate class NotRedundantFilePrivateClass { + fileprivate static func usedFilePrivateMethod() {} + + static func staticMethodCallingFilePrivate() { + usedFilePrivateMethod() + } +} + +/* + This separate type accesses NotRedundantFilePrivateClass. + Since they're in the same file, fileprivate allows the access. + If NotRedundantFilePrivateClass were private instead, this would fail to compile. +*/ +class NotRedundantFilePrivateClassUser { + func useFilePrivateClass() { + NotRedundantFilePrivateClass.staticMethodCallingFilePrivate() + } +} diff --git a/Tests/AccessibilityTests/AccessibilityProject/Sources/TargetA/NotRedundantFilePrivatePropertyInPrivateClass.swift b/Tests/AccessibilityTests/AccessibilityProject/Sources/TargetA/NotRedundantFilePrivatePropertyInPrivateClass.swift new file mode 100644 index 000000000..3673f0db4 --- /dev/null +++ b/Tests/AccessibilityTests/AccessibilityProject/Sources/TargetA/NotRedundantFilePrivatePropertyInPrivateClass.swift @@ -0,0 +1,37 @@ +/* + NotRedundantFilePrivatePropertyInPrivateClass.swift + Tests that a fileprivate property in a private class is NOT flagged as redundant + when accessed from a different type (extension) in the same file. + + This replicates the pattern from FilePath+Glob.swift where: + - A private class has a fileprivate property + - An extension of a different type accesses that property + - The fileprivate modifier is necessary for cross-type same-file access +*/ + +public extension SomePublicType { + static func accessPrivateClass() -> Set { + PrivateClassWithFilePrivateProperty().filePrivatePaths + } +} + +private class PrivateClassWithFilePrivateProperty { + fileprivate var filePrivatePaths: Set = ["path1", "path2"] + + func someMethod() { + _ = filePrivatePaths.count + } +} + +/* + This retainer ensures the extension method is used, making the file indexed. + The key test is that filePrivatePaths is accessed from the SomePublicType extension, + which is a different type than PrivateClassWithFilePrivateProperty. +*/ +public struct SomePublicType { + public init() {} + + public func retain() { + _ = SomePublicType.accessPrivateClass() + } +} diff --git a/Tests/AccessibilityTests/AccessibilityProject/Sources/TargetA/NotRedundantInternalClassComponents.swift b/Tests/AccessibilityTests/AccessibilityProject/Sources/TargetA/NotRedundantInternalClassComponents.swift new file mode 100644 index 000000000..5a6d60677 --- /dev/null +++ b/Tests/AccessibilityTests/AccessibilityProject/Sources/TargetA/NotRedundantInternalClassComponents.swift @@ -0,0 +1,27 @@ +// NotRedundantInternalClassComponents.swift +// Tests for internal classes/members that should NOT be flagged as redundant + +class NotRedundantInternalClass { + public init() {} + + internal func usedInternalMethod() {} +} + +internal struct NotRedundantInternalStruct { + internal var usedInternalProperty: Int = 0 + func useInternalProperty() -> Int { + return usedInternalProperty + } +} + +internal enum NotRedundantInternalEnum { + case usedCase + func useCase() -> NotRedundantInternalEnum { + return .usedCase + } +} + +// Test case for implicitly internal declaration used from another file - should NOT be flagged. +struct ImplicitlyInternalStructUsedFromAnotherFile { + var implicitlyInternalProperty: Int = 42 +} 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..87e672250 --- /dev/null +++ b/Tests/AccessibilityTests/AccessibilityProject/Sources/TargetA/NotRedundantInternalClassComponents_Support.swift @@ -0,0 +1,21 @@ +// NotRedundantInternalClassComponents_Support.swift +// Support types/usages for NotRedundantInternalClassComponents + +internal class NotRedundantInternalClass_Support { + internal func helper() {} +} + +// Used by main.swift to ensure these are referenced. +public class NotRedundantInternalClassComponents_Support { + public init() {} + + public func useInternalMethod() { + let cls = NotRedundantInternalClass() + cls.usedInternalMethod() + } + + public func useImplicitlyInternalStruct() { + let s = ImplicitlyInternalStructUsedFromAnotherFile() + _ = s.implicitlyInternalProperty + } +} \ No newline at end of file diff --git a/Tests/AccessibilityTests/AccessibilityProject/Sources/TargetA/PropertyWrapperAccessibility.swift b/Tests/AccessibilityTests/AccessibilityProject/Sources/TargetA/PropertyWrapperAccessibility.swift new file mode 100644 index 000000000..add913064 --- /dev/null +++ b/Tests/AccessibilityTests/AccessibilityProject/Sources/TargetA/PropertyWrapperAccessibility.swift @@ -0,0 +1,49 @@ +/* + PropertyWrapperAccessibility.swift + Tests that property wrapper members are NOT flagged as redundant. +*/ + +/* Property wrapper with internal accessibility. */ +@propertyWrapper +internal struct InternalPropertyWrapper { + private var value: T + + /* Should NOT be flagged - wrappedValue is part of property wrapper API. */ + internal var wrappedValue: T { + get { value } + set { value = newValue } + } + + /* Should NOT be flagged - projectedValue is part of property wrapper API. */ + internal var projectedValue: InternalPropertyWrapper { + self + } + + /* Should NOT be flagged - init is part of property wrapper API. */ + internal init(wrappedValue: T) { + value = wrappedValue + } + + /* This typealias is used in the init signature, so it's part of the API. */ + internal typealias Value = T +} + +/* Class using the property wrapper. */ +internal class ClassUsingPropertyWrapper { + @InternalPropertyWrapper + var wrappedProperty: String = "test" + + func access() { + _ = wrappedProperty + _ = $wrappedProperty + } +} + +/* Used by main.swift to ensure these are referenced. */ +public class PropertyWrapperAccessibilityRetainer { + public init() {} + public func retain() { + let obj = ClassUsingPropertyWrapper() + obj.access() + } +} diff --git a/Tests/AccessibilityTests/AccessibilityProject/Sources/TargetA/ProtocolRequirementAccessibility.swift b/Tests/AccessibilityTests/AccessibilityProject/Sources/TargetA/ProtocolRequirementAccessibility.swift new file mode 100644 index 000000000..20e214bc4 --- /dev/null +++ b/Tests/AccessibilityTests/AccessibilityProject/Sources/TargetA/ProtocolRequirementAccessibility.swift @@ -0,0 +1,41 @@ +/* + ProtocolRequirementAccessibility.swift + Tests that protocol requirements and conformances are NOT flagged as redundant. +*/ + +/* Internal protocol with internal requirements. */ +internal protocol InternalProtocolWithRequirements { + func requiredMethod() + var requiredProperty: Int { get } +} + +/* Class conforming to the internal protocol. */ +internal class ConformingToInternalProtocol: InternalProtocolWithRequirements { + /* Should NOT be flagged - this implements a protocol requirement. */ + func requiredMethod() {} + + /* Should NOT be flagged - this implements a protocol requirement. */ + var requiredProperty: Int { 42 } +} + +/* Protocol with fileprivate requirement (within same file). */ +fileprivate protocol FilePrivateProtocol { + func filePrivateRequirement() +} + +/* Extension conforming to fileprivate protocol. */ +extension ConformingToInternalProtocol: FilePrivateProtocol { + /* Should NOT be flagged - implements protocol requirement. */ + func filePrivateRequirement() {} +} + +/* Used by main.swift to ensure these are referenced. */ +public class ProtocolRequirementAccessibilityRetainer { + public init() {} + public func retain() { + let obj = ConformingToInternalProtocol() + obj.requiredMethod() + _ = obj.requiredProperty + obj.filePrivateRequirement() + } +} diff --git a/Tests/AccessibilityTests/AccessibilityProject/Sources/TargetA/RedundantFilePrivateClass.swift b/Tests/AccessibilityTests/AccessibilityProject/Sources/TargetA/RedundantFilePrivateClass.swift new file mode 100644 index 000000000..5b4d5e3f3 --- /dev/null +++ b/Tests/AccessibilityTests/AccessibilityProject/Sources/TargetA/RedundantFilePrivateClass.swift @@ -0,0 +1,31 @@ +/* + RedundantFilePrivateClass.swift + This file tests a case where fileprivate is actually NOT redundant because the + class is accessed from a different type (RedundantFilePrivateClassRetainer). +*/ + +fileprivate class RedundantFilePrivateClass { + fileprivate func someMethod() { + // Method that does something + } + + /* + This internal method creates a self-reference, ensuring the class is "used" + but only within its own declaration scope, making fileprivate redundant (could be private) + */ + func createInstance() -> RedundantFilePrivateClass { + RedundantFilePrivateClass() + } +} + +/* + This retainer accesses RedundantFilePrivateClass from a different type, + which means fileprivate is necessary (NOT redundant). +*/ +public class RedundantFilePrivateClassRetainer { + public init() {} + public func retain() { + let instance = RedundantFilePrivateClass() + _ = instance.createInstance() + } +} diff --git a/Tests/AccessibilityTests/AccessibilityProject/Sources/TargetA/RedundantInternalComponents.swift b/Tests/AccessibilityTests/AccessibilityProject/Sources/TargetA/RedundantInternalComponents.swift new file mode 100644 index 000000000..e45953e9d --- /dev/null +++ b/Tests/AccessibilityTests/AccessibilityProject/Sources/TargetA/RedundantInternalComponents.swift @@ -0,0 +1,82 @@ +// RedundantInternalComponents.swift +// Tests for internal classes/members that should be flagged as redundant + +internal class RedundantInternalClass { + internal func unusedInternalMethod() {} +} + +internal struct RedundantInternalStruct { + internal var unusedInternalProperty: Int = 0 +} + +internal enum RedundantInternalEnum { + case unusedCase +} + +// Test case for implicitly internal declaration that is redundant. +class ImplicitlyInternalClassUsedOnlyInSameFile { + var implicitlyInternalProperty: String = "test" + + func useProperty() { + _ = implicitlyInternalProperty + } +} + +// Retainer to ensure ImplicitlyInternalClassUsedOnlyInSameFile is used in the same file. +public class ImplicitlyInternalRetainer { + public init() {} + + public func retain() { + let obj = ImplicitlyInternalClassUsedOnlyInSameFile() + obj.useProperty() + _ = obj.implicitlyInternalProperty + } +} + +/* + Test case for members of private/fileprivate containers. + Members of a private class are already effectively private, + so marking them as redundant internal would be misleading. + */ +private class PrivateContainerClass { + // This should NOT be flagged - already constrained by parent's private accessibility. + func method() {} + + // This should NOT be flagged - already constrained by parent's private accessibility. + var property: Int = 0 +} + +fileprivate class FileprivateContainerClass { + // This should NOT be flagged - already constrained by parent's fileprivate accessibility. + func method() {} + + // This should NOT be flagged - already constrained by parent's fileprivate accessibility. + var property: Int = 0 +} + +/* + Test case for deinit - should not be flagged. + Deinitializers cannot have explicit access modifiers in Swift. + */ +class ClassWithDeinit { + // This should NOT be flagged - deinit cannot have access modifiers. + deinit {} +} + +/* + Test case for override methods - should not be flagged. + Override methods must be at least as accessible as what they override. + */ +class BaseClass { + func overridableMethod() {} +} + +class DerivedClass: BaseClass { + // This should NOT be flagged - override methods have accessibility constraints. + override func overridableMethod() {} +} + +// Used by main.swift to ensure these are referenced. +class RedundantInternalClassComponents { + public init() {} +} \ No newline at end of file diff --git a/Tests/AccessibilityTests/AccessibilityProject/Sources/TargetA/SameFileMemberwiseInit.swift b/Tests/AccessibilityTests/AccessibilityProject/Sources/TargetA/SameFileMemberwiseInit.swift new file mode 100644 index 000000000..bd2107cf2 --- /dev/null +++ b/Tests/AccessibilityTests/AccessibilityProject/Sources/TargetA/SameFileMemberwiseInit.swift @@ -0,0 +1,20 @@ +// Tests that struct memberwise init properties are NOT flagged when the init +// is used within the same file AND the struct is part of a type hierarchy +// that must remain internal. + +internal struct SameFileMemberwiseStruct { + let field1: String + let field2: Int +} + +internal struct SameFileOuterStruct { + let inner: SameFileMemberwiseStruct +} + +public class SameFileMemberwiseInitRetainer { + public init() {} + public func use() { + let inner = SameFileMemberwiseStruct(field1: "test", field2: 42) + _ = SameFileOuterStruct(inner: inner) + } +} diff --git a/Tests/AccessibilityTests/AccessibilityProject/Sources/TargetA/SameFileMemberwiseInit_Consumer.swift b/Tests/AccessibilityTests/AccessibilityProject/Sources/TargetA/SameFileMemberwiseInit_Consumer.swift new file mode 100644 index 000000000..44b018e85 --- /dev/null +++ b/Tests/AccessibilityTests/AccessibilityProject/Sources/TargetA/SameFileMemberwiseInit_Consumer.swift @@ -0,0 +1,6 @@ +class SameFileMemberwiseInitConsumer { + func consume() { + let inner = SameFileMemberwiseStruct(field1: "test", field2: 42) + _ = SameFileOuterStruct(inner: inner) + } +} diff --git a/Tests/AccessibilityTests/AccessibilityProject/Sources/TargetA/SameFileTypeConstraint.swift b/Tests/AccessibilityTests/AccessibilityProject/Sources/TargetA/SameFileTypeConstraint.swift new file mode 100644 index 000000000..edd864004 --- /dev/null +++ b/Tests/AccessibilityTests/AccessibilityProject/Sources/TargetA/SameFileTypeConstraint.swift @@ -0,0 +1,59 @@ +// Tests that types used in signatures of other internal types are NOT flagged +// when the containing type must remain internal. + +// MARK: - Property Type Constraint + +internal enum SameFileConstrainedEnum { + case one + case two +} + +internal struct SameFileConstrainingStruct { + let enumValue: SameFileConstrainedEnum +} + +// MARK: - Return Type Constraint + +internal struct SameFileReturnType { + var value: Int = 0 +} + +internal class SameFileClassWithReturnType { + func getReturnType() -> SameFileReturnType { + SameFileReturnType(value: 42) + } +} + +// MARK: - Parameter Type Constraint + +internal struct SameFileParamType { + var data: String = "" +} + +internal class SameFileClassWithParamType { + func process(_ param: SameFileParamType) { + _ = param.data + } +} + +// MARK: - Generic Constraint + +internal protocol SameFileConstraintProtocol { + var id: String { get } +} + +internal class SameFileClassWithGenericConstraint { + func process(_ item: T) -> String { + item.id + } +} + +public class SameFileTypeConstraintRetainer { + public init() {} + public func use() { + _ = SameFileConstrainingStruct(enumValue: .one) + _ = SameFileClassWithReturnType() + _ = SameFileClassWithParamType() + _ = SameFileClassWithGenericConstraint() + } +} diff --git a/Tests/AccessibilityTests/AccessibilityProject/Sources/TargetA/SameFileTypeConstraint_Consumer.swift b/Tests/AccessibilityTests/AccessibilityProject/Sources/TargetA/SameFileTypeConstraint_Consumer.swift new file mode 100644 index 000000000..b09551f67 --- /dev/null +++ b/Tests/AccessibilityTests/AccessibilityProject/Sources/TargetA/SameFileTypeConstraint_Consumer.swift @@ -0,0 +1,15 @@ +class SameFileTypeConstraintConsumer { + func consumePropertyType() { + _ = SameFileConstrainingStruct(enumValue: .one) + } + + func consumeReturnType() { + let obj = SameFileClassWithReturnType() + _ = obj.getReturnType() + } + + func consumeParamType() { + let obj = SameFileClassWithParamType() + obj.process(SameFileParamType(data: "test")) + } +} diff --git a/Tests/AccessibilityTests/AccessibilityProject/Sources/TargetA/StoredPropertyTypeExposure.swift b/Tests/AccessibilityTests/AccessibilityProject/Sources/TargetA/StoredPropertyTypeExposure.swift new file mode 100644 index 000000000..4b8add0e7 --- /dev/null +++ b/Tests/AccessibilityTests/AccessibilityProject/Sources/TargetA/StoredPropertyTypeExposure.swift @@ -0,0 +1,71 @@ +// StoredPropertyTypeExposure.swift +// Test cases for types used as stored property types that are transitively exposed. +// +// When a type T is used as a property type in a struct/class C, and C is instantiated +// from another file, T is transitively exposed and should NOT be flagged as redundant internal. + +// MARK: - Simple Property Type Exposure + +// StoredPropertyRole is used as the property type in StoredPropertyContainer. +// StoredPropertyContainer is instantiated from StoredPropertyTypeExposure_Consumer.swift. +// Therefore StoredPropertyRole is transitively exposed and should NOT be flagged. +internal enum StoredPropertyRole { + case primary + case secondary +} + +internal struct StoredPropertyContainer { + let role: StoredPropertyRole +} + +// MARK: - Nested Type as Property Type + +// NestedPhase is a nested enum used as a property type in its containing class. +// ClassWithNestedType is instantiated from StoredPropertyTypeExposure_Consumer.swift. +// Therefore NestedPhase is transitively exposed and should NOT be flagged. +internal class ClassWithNestedType { + internal enum NestedPhase { + case idle + case running + case completed + } + + var phase: NestedPhase = .idle + + func advance() { + switch phase { + case .idle: phase = .running + case .running: phase = .completed + case .completed: break + } + } +} + +// MARK: - Chained Property Type Exposure + +// InnerType is used in MiddleContainer, which is used in OuterContainer. +// OuterContainer is instantiated from StoredPropertyTypeExposure_Consumer.swift. +// Therefore both InnerType and MiddleContainer are transitively exposed. +internal struct InnerType { + var value: Int = 0 +} + +internal struct MiddleContainer { + var inner: InnerType +} + +internal struct OuterContainer { + var middle: MiddleContainer +} + +// MARK: - Retainer class to ensure all code is exercised + +public class StoredPropertyTypeExposureRetainer { + public init() {} + + public func use() { + _ = StoredPropertyContainer(role: .primary) + _ = ClassWithNestedType() + _ = OuterContainer(middle: MiddleContainer(inner: InnerType(value: 42))) + } +} diff --git a/Tests/AccessibilityTests/AccessibilityProject/Sources/TargetA/StoredPropertyTypeExposure_Consumer.swift b/Tests/AccessibilityTests/AccessibilityProject/Sources/TargetA/StoredPropertyTypeExposure_Consumer.swift new file mode 100644 index 000000000..9b18f4920 --- /dev/null +++ b/Tests/AccessibilityTests/AccessibilityProject/Sources/TargetA/StoredPropertyTypeExposure_Consumer.swift @@ -0,0 +1,24 @@ +// StoredPropertyTypeExposure_Consumer.swift +// Consumer file that references types from StoredPropertyTypeExposure.swift, +// creating cross-file transitive exposure of the property types. + +class StoredPropertyTypeExposureConsumer { + // Uses StoredPropertyContainer, which transitively exposes StoredPropertyRole + func consumeSimplePropertyType() { + let container = StoredPropertyContainer(role: .primary) + _ = container.role + } + + // Uses ClassWithNestedType, which transitively exposes NestedPhase + func consumeNestedType() { + let obj = ClassWithNestedType() + obj.advance() + _ = obj.phase + } + + // Uses OuterContainer, which transitively exposes MiddleContainer and InnerType + func consumeChainedPropertyTypes() { + let outer = OuterContainer(middle: MiddleContainer(inner: InnerType(value: 100))) + _ = outer.middle.inner.value + } +} diff --git a/Tests/AccessibilityTests/AccessibilityProject/Sources/TargetA/StructMemberwiseInitAccessibility.swift b/Tests/AccessibilityTests/AccessibilityProject/Sources/TargetA/StructMemberwiseInitAccessibility.swift new file mode 100644 index 000000000..4d2ed9fe7 --- /dev/null +++ b/Tests/AccessibilityTests/AccessibilityProject/Sources/TargetA/StructMemberwiseInitAccessibility.swift @@ -0,0 +1,53 @@ +/* + StructMemberwiseInitAccessibility.swift + Tests that struct stored properties used in implicit memberwise initializers + are NOT flagged as redundant internal. + + When a struct relies on its implicit memberwise initializer, the stored + properties that are parameters to that initializer are part of the struct's + public API and must maintain their accessibility. +*/ + +// Struct with internal stored properties used in memberwise init. +// These properties should NOT be flagged as redundant internal. +public struct StructWithMemberwiseInit { + internal var memberwiseProperty1: String + internal var memberwiseProperty2: Int + internal let memberwiseConstant: Bool + + // No explicit init - relies on implicit memberwise init +} + +// Struct with mixed properties - some in memberwise init, some not. +public struct StructWithMixedProperties { + internal var memberwiseProperty: String + + // Computed property - not part of memberwise init + internal var computedProperty: String { memberwiseProperty.uppercased() } + + // Property with default value - still part of memberwise init + internal var propertyWithDefault: Int = 42 + + // No explicit init - relies on implicit memberwise init +} + +// Usage within the file to exercise the structs. +public class StructMemberwiseInitAccessibilityRetainer { + public init() {} + + public func retain() { + // Using memberwise initializer + let s1 = StructWithMemberwiseInit( + memberwiseProperty1: "hello", + memberwiseProperty2: 42, + memberwiseConstant: true + ) + _ = s1 + + let s2 = StructWithMixedProperties( + memberwiseProperty: "world", + propertyWithDefault: 100 + ) + _ = s2.computedProperty + } +} diff --git a/Tests/AccessibilityTests/AccessibilityProject/Sources/TargetA/TransitiveAccessExposure.swift b/Tests/AccessibilityTests/AccessibilityProject/Sources/TargetA/TransitiveAccessExposure.swift new file mode 100644 index 000000000..61dad9c51 --- /dev/null +++ b/Tests/AccessibilityTests/AccessibilityProject/Sources/TargetA/TransitiveAccessExposure.swift @@ -0,0 +1,267 @@ +// TransitiveAccessExposure.swift +// Comprehensive test cases for transitive access exposure scenarios. +// Each scenario shows a type that must have sufficient access level because +// it is exposed through another declaration's signature. +// +// For each case: +// - The inner type is marked with the minimum required access level +// - A comment shows what error would occur if we lowered the access level +// +// NOTE: This file focuses on CROSS-FILE exposure scenarios (internal types). +// Same-file scenarios (fileprivate types) are handled in a separate file. + +// MARK: - 1. Function Parameter Type Exposure + +// Scenario 1a: Internal type used as parameter of internal function called from another file +// If ParameterTypeA were fileprivate: +// "Method must be declared fileprivate because its parameter uses a fileprivate type" +internal struct ParameterTypeA { + var value: Int = 0 +} + +class ParameterExposureContainer { + // This function is called from TransitiveAccessExposure_Consumer.swift + func processParameter(_ param: ParameterTypeA) { + _ = param.value + } +} + +// MARK: - 2. Function Return Type Exposure + +// Scenario 2a: Internal type used as return type of function called from another file +// If ReturnTypeA were fileprivate: +// "Function must be declared fileprivate because its result uses a fileprivate type" +internal enum ReturnTypeA { + case success + case failure +} + +class ReturnTypeExposureContainer { + // This function is called from TransitiveAccessExposure_Consumer.swift + func getResult() -> ReturnTypeA { + .success + } +} + +// MARK: - 3. Property Type Exposure + +// Scenario 3a: Internal type used as property type, property accessed from another file +// If PropertyTypeA were fileprivate: +// "Property must be declared fileprivate because its type uses a fileprivate type" +internal struct PropertyTypeA { + var data: String = "" +} + +class PropertyTypeExposureContainer { + // This property is accessed from TransitiveAccessExposure_Consumer.swift + var exposedProperty: PropertyTypeA = PropertyTypeA() +} + +// MARK: - 4. Generic Constraint Exposure + +// Scenario 4a: Internal protocol used as generic constraint, function called from another file +// If GenericConstraintProtocolA were fileprivate: +// "Generic parameter 'T' cannot be declared internal because it uses a fileprivate type in its requirement" +internal protocol GenericConstraintProtocolA { + var identifier: String { get } +} + +internal struct GenericConstraintConformingTypeA: GenericConstraintProtocolA { + var identifier: String = "A" +} + +class GenericConstraintExposureContainer { + // This function is called from TransitiveAccessExposure_Consumer.swift + func processGeneric(_ item: T) -> String { + item.identifier + } +} + +// MARK: - 5. Protocol Requirement Exposure +// Note: Protocol requirements expose types in their signatures + +// Scenario 5a: Internal type used in protocol requirement parameter +// If ProtocolRequirementTypeA were fileprivate: +// "Method in an internal protocol cannot use a fileprivate type" +internal struct ProtocolRequirementTypeA { + var payload: Int = 0 +} + +internal protocol ProtocolWithRequirementA { + func process(input: ProtocolRequirementTypeA) +} + +class ProtocolRequirementExposureContainer: ProtocolWithRequirementA { + func process(input: ProtocolRequirementTypeA) { + _ = input.payload + } +} + +// MARK: - 6. Enum Associated Value Exposure + +// Scenario 6a: Internal type used as enum associated value, enum used from another file +// If EnumAssociatedTypeA were fileprivate: +// "Enum case in an internal enum uses a fileprivate type" +internal struct EnumAssociatedTypeA { + var content: String = "" +} + +internal enum EnumWithAssociatedValueA { + case success(EnumAssociatedTypeA) + case failure(Error) +} + +class EnumAssociatedValueExposureContainer { + // This function is called from TransitiveAccessExposure_Consumer.swift + func getEnumValue() -> EnumWithAssociatedValueA { + .success(EnumAssociatedTypeA(content: "data")) + } +} + +// MARK: - 7. Typealias Exposure + +// Scenario 7a: Internal type aliased by internal typealias, used from another file +// If TypealiasTargetTypeA were fileprivate: +// "Type alias cannot be declared internal because it uses a fileprivate type" +internal struct TypealiasTargetTypeA { + var value: Double = 0.0 +} + +internal typealias AliasedTypeA = TypealiasTargetTypeA + +class TypealiasExposureContainer { + // This property uses the typealias and is accessed from TransitiveAccessExposure_Consumer.swift + var aliasedProperty: AliasedTypeA = AliasedTypeA() +} + +// MARK: - 8. Subscript Exposure + +// Scenario 8a: Internal type used as subscript parameter, subscript accessed from another file +// If SubscriptKeyTypeA were fileprivate: +// "Subscript cannot be declared internal because its parameter uses a fileprivate type" +internal struct SubscriptKeyTypeA: Hashable { + var key: String = "" +} + +class SubscriptExposureContainer { + private var storage: [SubscriptKeyTypeA: Int] = [:] + + // This subscript is accessed from TransitiveAccessExposure_Consumer.swift + subscript(key: SubscriptKeyTypeA) -> Int { + get { storage[key] ?? 0 } + set { storage[key] = newValue } + } +} + +// Scenario 8b: Internal type used as subscript return type +// If SubscriptReturnTypeA were fileprivate: +// "Subscript cannot be declared internal because its element type uses a fileprivate type" +internal struct SubscriptReturnTypeA { + var data: String = "" +} + +class SubscriptReturnTypeExposureContainer { + private var items: [SubscriptReturnTypeA] = [] + + // This subscript is accessed from TransitiveAccessExposure_Consumer.swift + subscript(index: Int) -> SubscriptReturnTypeA { + items.indices.contains(index) ? items[index] : SubscriptReturnTypeA() + } +} + +// MARK: - 9. Default Argument Exposure + +// Scenario 9a: Internal type used in default argument, function called from another file +// If DefaultArgTypeA were fileprivate: +// "Default argument value of internal function uses a fileprivate type" +internal struct DefaultArgTypeA { + var config: String = "default" + static let defaultValue = DefaultArgTypeA() +} + +class DefaultArgumentExposureContainer { + // This function is called from TransitiveAccessExposure_Consumer.swift + func processWithDefault(config: DefaultArgTypeA = DefaultArgTypeA.defaultValue) { + _ = config.config + } +} + +// MARK: - 10. Initializer Parameter Exposure + +// Scenario 10a: Internal type used as initializer parameter, init called from another file +// If InitParamTypeA were fileprivate: +// "Initializer cannot be declared internal because its parameter uses a fileprivate type" +internal struct InitParamTypeA { + var setting: Bool = false +} + +class InitializerExposureContainer { + let config: InitParamTypeA + + // This initializer is called from TransitiveAccessExposure_Consumer.swift + init(config: InitParamTypeA) { + self.config = config + } +} + +// MARK: - 11. Closure Type Exposure + +// Scenario 11a: Internal type used in closure parameter/return, closure accessed from another file +// If ClosureParamTypeA were fileprivate: +// "Property cannot be declared internal because its type uses a fileprivate type" +internal struct ClosureParamTypeA { + var input: Int = 0 +} + +internal struct ClosureReturnTypeA { + var output: Int = 0 +} + +class ClosureTypeExposureContainer { + // This closure property is accessed from TransitiveAccessExposure_Consumer.swift + var transformer: (ClosureParamTypeA) -> ClosureReturnTypeA = { param in + ClosureReturnTypeA(output: param.input * 2) + } +} + +// MARK: - Retainer class to ensure all code is exercised + +public class TransitiveAccessExposureRetainer { + public init() {} + + public func retain() { + // 1. Parameter exposure + _ = ParameterExposureContainer() + + // 2. Return type exposure + _ = ReturnTypeExposureContainer() + + // 3. Property type exposure + _ = PropertyTypeExposureContainer() + + // 4. Generic constraint exposure + _ = GenericConstraintExposureContainer() + + // 5. Protocol requirement exposure + _ = ProtocolRequirementExposureContainer() + + // 6. Enum associated value exposure + _ = EnumAssociatedValueExposureContainer() + + // 7. Typealias exposure + _ = TypealiasExposureContainer() + + // 8. Subscript exposure + _ = SubscriptExposureContainer() + _ = SubscriptReturnTypeExposureContainer() + + // 9. Default argument exposure + _ = DefaultArgumentExposureContainer() + + // 10. Initializer exposure + _ = InitializerExposureContainer(config: InitParamTypeA()) + + // 11. Closure type exposure + _ = ClosureTypeExposureContainer() + } +} diff --git a/Tests/AccessibilityTests/AccessibilityProject/Sources/TargetA/TransitiveAccessExposure_Consumer.swift b/Tests/AccessibilityTests/AccessibilityProject/Sources/TargetA/TransitiveAccessExposure_Consumer.swift new file mode 100644 index 000000000..ca2c400f6 --- /dev/null +++ b/Tests/AccessibilityTests/AccessibilityProject/Sources/TargetA/TransitiveAccessExposure_Consumer.swift @@ -0,0 +1,94 @@ +// TransitiveAccessExposure_Consumer.swift +// Consumer file that references declarations from TransitiveAccessExposure.swift +// This creates cross-file transitive exposure of the inner types. + +class TransitiveAccessExposureConsumer { + // 1. Parameter exposure - calls function with ParameterTypeA parameter + func consumeParameterExposure() { + let container = ParameterExposureContainer() + container.processParameter(ParameterTypeA(value: 42)) + } + + // 2. Return type exposure - calls function returning ReturnTypeA + func consumeReturnTypeExposure() { + let container = ReturnTypeExposureContainer() + let result = container.getResult() + switch result { + case .success: break + case .failure: break + } + } + + // 3. Property type exposure - accesses property of PropertyTypeA type + func consumePropertyTypeExposure() { + let container = PropertyTypeExposureContainer() + _ = container.exposedProperty + } + + // 4. Generic constraint exposure - calls generic function constrained by GenericConstraintProtocolA + func consumeGenericConstraintExposure() { + let container = GenericConstraintExposureContainer() + let item = GenericConstraintConformingTypeA() + _ = container.processGeneric(item) + } + + // 5. Protocol requirement exposure - uses conforming type + func consumeProtocolRequirementExposure() { + let container = ProtocolRequirementExposureContainer() + container.process(input: ProtocolRequirementTypeA(payload: 100)) + } + + // 6. Enum associated value exposure - uses enum with associated type + func consumeEnumAssociatedValueExposure() { + let container = EnumAssociatedValueExposureContainer() + let value = container.getEnumValue() + switch value { + case .success(let data): _ = data.content + case .failure: break + } + } + + // 7. Typealias exposure - uses typealias property + func consumeTypealiasExposure() { + let container = TypealiasExposureContainer() + _ = container.aliasedProperty + } + + // 8a. Subscript key exposure - uses subscript with SubscriptKeyTypeA + func consumeSubscriptKeyExposure() { + var container = SubscriptExposureContainer() + let key = SubscriptKeyTypeA(key: "test") + container[key] = 10 + _ = container[key] + } + + // 8b. Subscript return type exposure - uses subscript returning SubscriptReturnTypeA + func consumeSubscriptReturnTypeExposure() { + let container = SubscriptReturnTypeExposureContainer() + let item: SubscriptReturnTypeA = container[0] + _ = item.data + } + + // 9. Default argument exposure - calls function with default argument + func consumeDefaultArgumentExposure() { + let container = DefaultArgumentExposureContainer() + // Calling with default argument + container.processWithDefault() + // Calling with explicit argument + container.processWithDefault(config: DefaultArgTypeA(config: "custom")) + } + + // 10. Initializer exposure - calls initializer with InitParamTypeA + func consumeInitializerExposure() { + let config = InitParamTypeA(setting: true) + _ = InitializerExposureContainer(config: config) + } + + // 11. Closure type exposure - uses closure with exposed types + func consumeClosureTypeExposure() { + let container = ClosureTypeExposureContainer() + let input = ClosureParamTypeA(input: 5) + let output = container.transformer(input) + _ = output.output + } +} diff --git a/Tests/AccessibilityTests/AccessibilityProject/Sources/TargetA/TrulyRedundantFilePrivateMembers.swift b/Tests/AccessibilityTests/AccessibilityProject/Sources/TargetA/TrulyRedundantFilePrivateMembers.swift new file mode 100644 index 000000000..7c98a6e82 --- /dev/null +++ b/Tests/AccessibilityTests/AccessibilityProject/Sources/TargetA/TrulyRedundantFilePrivateMembers.swift @@ -0,0 +1,38 @@ +/* + TrulyRedundantFilePrivateMembers.swift + Tests cases where fileprivate is truly redundant and should be private. +*/ + +/* This class has a fileprivate method that is only used within the class itself. */ +class ClassWithRedundantFilePrivateMethod { + /* Should be flagged as redundant - only used within the same type (can be private). */ + fileprivate func helper() -> Int { + 42 + } + + func publicMethod() -> Int { + helper() + } +} + +/* This struct has a fileprivate property only accessed within its own type. */ +struct StructWithRedundantFilePrivateProperty { + /* Should be flagged as redundant - only used within the same type (can be private). */ + fileprivate var internalState: String = "" + + func access() -> String { + internalState + } +} + +/* Used by main.swift to ensure these are referenced. */ +public class TrulyRedundantFilePrivateMembersRetainer { + public init() {} + public func retain() { + let obj = ClassWithRedundantFilePrivateMethod() + _ = obj.publicMethod() + + let s = StructWithRedundantFilePrivateProperty() + _ = s.access() + } +} diff --git a/Tests/AccessibilityTests/AccessibilityProject/Sources/TargetA/UsableFromInlineAccessibility.swift b/Tests/AccessibilityTests/AccessibilityProject/Sources/TargetA/UsableFromInlineAccessibility.swift new file mode 100644 index 000000000..50689f712 --- /dev/null +++ b/Tests/AccessibilityTests/AccessibilityProject/Sources/TargetA/UsableFromInlineAccessibility.swift @@ -0,0 +1,46 @@ +// Test file for @usableFromInline attribute handling + +public struct PublicContainer { + // This should NOT be flagged as redundant internal because it has @usableFromInline. + // @usableFromInline requires the declaration to remain internal (or package) so it can + // be inlined into client code. + @usableFromInline + internal init() {} + + // This should NOT be flagged as redundant internal because of @usableFromInline. + @usableFromInline + internal func inlinableHelper() -> Int { + 42 + } + + // This should NOT be flagged as redundant internal because of @usableFromInline. + @usableFromInline + internal var inlinableProperty: String { + "value" + } + + // This should NOT be flagged as redundant internal. + // Even though it's only used in this file, @usableFromInline means it could be + // inlined into client code that imports this module. + @usableFromInline + internal static func inlinableStaticMethod() -> Bool { + true + } + + // Public method that uses the @usableFromInline members + @inlinable + public func publicInlinableMethod() -> String { + "\(inlinableProperty): \(inlinableHelper())" + } +} + +// This SHOULD be flagged as redundant internal - no @usableFromInline attribute +// and only used within the same file in a non-inlinable private function. +internal func regularInternalMethod() -> String { + PublicContainer.inlinableStaticMethod().description +} + +// Use the regular internal method within the same file +private func useRegularMethod() { + _ = regularInternalMethod() +} diff --git a/Tests/AccessibilityTests/AccessibilityProject/Tests/TestTarget/InternalTestableImportTest.swift b/Tests/AccessibilityTests/AccessibilityProject/Tests/TestTarget/InternalTestableImportTest.swift new file mode 100644 index 000000000..09d5901e9 --- /dev/null +++ b/Tests/AccessibilityTests/AccessibilityProject/Tests/TestTarget/InternalTestableImportTest.swift @@ -0,0 +1,14 @@ +import Foundation +import XCTest +@testable import TargetA + +class InternalTestableImportTest: XCTestCase { + func testInternalAccess() { + // Access internal declarations via @testable import + let obj1 = InternalUsedOnlyInTest() + obj1.testOnlyMethod() + + let obj2 = InternalUsedInBoth() + obj2.sharedMethod() + } +} diff --git a/Tests/AccessibilityTests/RedundantFilePrivateAccessibilityTest.swift b/Tests/AccessibilityTests/RedundantFilePrivateAccessibilityTest.swift new file mode 100644 index 000000000..272238585 --- /dev/null +++ b/Tests/AccessibilityTests/RedundantFilePrivateAccessibilityTest.swift @@ -0,0 +1,58 @@ +import Configuration +@testable import TestShared +import XCTest + +final class RedundantFilePrivateAccessibilityTest: SPMSourceGraphTestCase { + override static func setUp() { + super.setUp() + build(projectPath: AccessibilityProjectPath) + } + + // Tests that a fileprivate class is NOT flagged as redundant when accessed + // from a different type in the same file. + // + // In Swift, private and fileprivate have distinct meanings even for top-level declarations: + // - private: accessible only within the lexical scope and its extensions in the same file + // - fileprivate: accessible from anywhere in the same file + // + // Since RedundantFilePrivateClass is accessed from RedundantFilePrivateClassRetainer + // (a different type), fileprivate is the minimum access level required. Changing it to + // private would prevent RedundantFilePrivateClassRetainer from accessing it. + func testRedundantFilePrivateClass() { + index() + assertNotRedundantFilePrivateAccessibility(.class("RedundantFilePrivateClass")) + } + + func testNotRedundantFilePrivateClass() { + index() + assertNotRedundantFilePrivateAccessibility(.class("NotRedundantFilePrivateClass")) + } + + // Tests that a fileprivate property inside a private class is NOT flagged as redundant. + // This is valid because fileprivate expands the property's visibility beyond the private + // container, making it accessible to other types in the same file. The private class + // restricts access to the file, but fileprivate allows the property to be more visible + // than its container within that scope. + func testNotRedundantFilePrivatePropertyInPrivateClass() { + index() + assertNotRedundantFilePrivateAccessibility(.varInstance("filePrivatePaths")) + } + + func testTrulyRedundantFilePrivateMethod() { + // A fileprivate method only used within its own type should be private. + index() + assertRedundantFilePrivateAccessibility( + .functionMethodInstance("helper()", line: 9), + containingTypeName: "class ClassWithRedundantFilePrivateMethod" + ) + } + + func testTrulyRedundantFilePrivateProperty() { + // A fileprivate property only used within its own type should be private. + index() + assertRedundantFilePrivateAccessibility( + .varInstance("internalState"), + containingTypeName: "struct StructWithRedundantFilePrivateProperty" + ) + } +} diff --git a/Tests/AccessibilityTests/RedundantInternalAccessibilityTest.swift b/Tests/AccessibilityTests/RedundantInternalAccessibilityTest.swift new file mode 100644 index 000000000..ef16222c1 --- /dev/null +++ b/Tests/AccessibilityTests/RedundantInternalAccessibilityTest.swift @@ -0,0 +1,525 @@ +import Configuration +@testable import TestShared +import XCTest + +final class RedundantInternalAccessibilityTest: SPMSourceGraphTestCase { + override static func setUp() { + super.setUp() + build(projectPath: AccessibilityProjectPath) + } + + func testInternalPropertyUsedInExtensionInOtherFile() { + // This should NOT be flagged as redundant + // Tests the case where an internal property is used in an extension in a different file + index() + + // InternalPropertyUsedInExtension.propertyUsedInExtension should NOT be flagged as redundant + // because it's used in InternalPropertyExtension.swift + assertNotRedundantInternalAccessibility(.varInstance("propertyUsedInExtension")) + } + + func testInternalPropertyUsedOnlyInSameFile() { + // This should be flagged as redundant + // Tests the case where an internal property is only used within its own file + index() + + // InternalPropertyUsedInExtension.propertyOnlyUsedInSameFile should be flagged as redundant + // because it's only used within InternalPropertyUsedInExtension.swift + assertRedundantInternalAccessibility(.varInstance("propertyOnlyUsedInSameFile")) + } + + func testInternalPropertyUsedInMultipleFiles() { + // This should NOT be flagged as redundant + // Tests the case where an internal property is used across multiple files + index() + + // This test would need additional setup with multiple files + // For now, we'll test that the existing NotRedundantInternalClassComponents work + assertNotRedundantInternalAccessibility(.class("NotRedundantInternalClass")) + } + + func testInternalMethodUsedInExtension() { + // This should NOT be flagged as redundant. + index() + + assertNotRedundantInternalAccessibility(.functionMethodInstance("usedInternalMethod()")) + } + + /// Tests that members of a private class are not flagged as redundant internal. + /// + /// In Swift, members of a private class are already effectively private due to + /// the parent's accessibility constraint. Suggesting to change them to fileprivate + /// would actually increase their visibility, which is incorrect. + func testMembersOfPrivateClassNotFlagged() { + index() + + assertNotRedundantInternalAccessibility(.functionMethodInstance("method()", line: 43)) + assertNotRedundantInternalAccessibility(.varInstance("property", line: 46)) + } + + /// Tests that members of a fileprivate class are not flagged as redundant internal + /// when they would be suggested to be fileprivate. + /// + /// In Swift, members of a fileprivate class are already constrained to fileprivate + /// accessibility. Suggesting to mark them as fileprivate would be redundant. + func testMembersOfFileprivateClassNotFlagged() { + index() + + assertNotRedundantInternalAccessibility(.functionMethodInstance("method()", line: 51)) + assertNotRedundantInternalAccessibility(.varInstance("property", line: 54)) + } + + /// Tests that deinit is not flagged as redundant internal. + /// + /// In Swift, deinitializers cannot have explicit access modifiers. + /// They always match the accessibility of their enclosing type. + func testDeinitNotFlagged() { + index() + + assertNotRedundantInternalAccessibility(.functionDestructor("deinit", line: 63)) + } + + /// Tests that override methods are not flagged as redundant internal. + /// + /// Override methods must be at least as accessible as the method they override, + /// so their accessibility is constrained by the base method. + func testOverrideMethodNotFlagged() { + index() + + assertNotRedundantInternalAccessibility(.functionMethodInstance("overridableMethod()", line: 76)) + } + + /// Tests that protocol requirements are not flagged as redundant internal. + /// + /// Methods and properties that implement protocol requirements must maintain + /// sufficient accessibility to fulfill the protocol contract. + func testProtocolRequirementsNotFlagged() { + index() + + assertNotRedundantInternalAccessibility(.functionMethodInstance("requiredMethod()")) + assertNotRedundantInternalAccessibility(.varInstance("requiredProperty")) + } + + /// Tests that fileprivate protocol conformances are not flagged. + func testFilePrivateProtocolConformanceNotFlagged() { + index() + + assertNotRedundantInternalAccessibility(.functionMethodInstance("filePrivateRequirement()")) + } + + /// Tests that property wrapper members are not flagged as redundant internal. + /// + /// Property wrappers require certain members (init, wrappedValue, projectedValue) + /// to be accessible as part of their API contract. + func testPropertyWrapperMembersNotFlagged() { + index() + + assertNotRedundantInternalAccessibility(.varInstance("wrappedValue")) + assertNotRedundantInternalAccessibility(.varInstance("projectedValue")) + assertNotRedundantInternalAccessibility(.functionConstructor("init(wrappedValue:)")) + } + + /// Tests internal suggesting private. + /// + /// An internal property only used within its own type should be private. + func testInternalSuggestingPrivate() { + index() + + assertRedundantInternalAccessibility( + .varInstance("usedOnlyInOwnType"), + suggestedAccessibility: .private + ) + } + + /// Tests internal suggesting fileprivate. + /// + /// An internal property used from a different type in the same file should be fileprivate. + func testInternalSuggestingFileprivate() { + index() + + assertRedundantInternalAccessibility( + .varInstance("sharedWithClassB"), + suggestedAccessibility: .fileprivate + ) + } + + /// Tests nested type redundancy with parent already flagged. + /// + /// When nested types are flagged as redundant, their nested members should be + /// suppressed to avoid noise (unless showNestedRedundantAccessibility is enabled). + func testNestedTypeRedundancy() { + index() + + assertRedundantInternalAccessibility(.struct("NestedStruct")) + assertRedundantInternalAccessibility(.class("NestedClass")) + assertRedundantInternalAccessibility(.varInstance("nested")) + } + + /// Tests that implicitly internal declarations (no access modifier) are flagged as redundant + /// when only used within the same file. + /// + /// This ensures the analyzer handles both explicit 'internal' keyword and implicit internal + /// (default accessibility) correctly in positive cases. + func testImplicitlyInternalRedundant() { + index() + + assertRedundantInternalAccessibility(.class("ImplicitlyInternalClassUsedOnlyInSameFile")) + } + + /// Tests that implicitly internal declarations (no access modifier) are NOT flagged as redundant + /// when used from another file. + /// + /// This ensures the analyzer handles both explicit 'internal' keyword and implicit internal + /// (default accessibility) correctly in negative cases. + func testImplicitlyInternalNotRedundant() { + index() + + assertNotRedundantInternalAccessibility(.struct("ImplicitlyInternalStructUsedFromAnotherFile")) + } + + /// Tests that internal declarations accessed from test files via @testable import + /// are NOT flagged as redundant internal. + /// + /// This verifies that @testable import references count as legitimate cross-file usage, + /// preventing false positives when test files access internal members. Since tests ARE + /// using these internal members from a different file, they require internal accessibility. + func testInternalUsedViaTestableImportNotFlagged() { + index() + + // InternalUsedOnlyInTest should NOT be flagged because it IS used from + // a test file (different file) via @testable import + assertNotRedundantInternalAccessibility(.class("InternalUsedOnlyInTest")) + } + + /// Tests that internal declarations used from production code in the same module + /// are NOT flagged as redundant (baseline behavior verification). + func testInternalUsedInProductionNotFlagged() { + index() + + // InternalUsedInBoth should NOT be flagged because it's used from + // production code (InternalTestableImportUsage_Support.swift) within the same module + assertNotRedundantInternalAccessibility(.class("InternalUsedInBoth")) + } + + /// Tests that declarations with @usableFromInline are NOT flagged as redundant internal. + /// + /// The @usableFromInline attribute allows internal declarations to be inlined into + /// client code, requiring them to maintain internal (or package) visibility. Marking + /// them as fileprivate or private would cause a compiler error because @usableFromInline + /// is incompatible with those access levels. + /// + /// This test verifies the fix for a build error on Linux where @usableFromInline + /// declarations were incorrectly flagged as redundant internal, and changing them + /// to fileprivate caused: "@usableFromInline attribute can only be applied to + /// internal or package declarations". + func testUsableFromInlineNotFlagged() { + index() + + // All @usableFromInline members should NOT be flagged, even if only used in same file + assertNotRedundantInternalAccessibility(.functionConstructor("init()")) + assertNotRedundantInternalAccessibility(.functionMethodInstance("inlinableHelper()")) + assertNotRedundantInternalAccessibility(.varInstance("inlinableProperty")) + assertNotRedundantInternalAccessibility(.functionMethodStatic("inlinableStaticMethod()")) + } + + /// Tests that internal types conforming to external protocols are NOT flagged as redundant. + /// + /// When a type conforms to an external protocol (from another module), its protocol + /// requirement implementations must maintain their accessibility to fulfill the + /// protocol contract. This prevents false positives like those seen with + /// CheckUpdateCommand, ScanCommand, etc. implementing ArgumentParser's ParsableCommand. + func testExternalProtocolConformanceNotFlagged() { + index() + + // Internal struct conforming to ExternalProtocol should NOT be flagged + assertNotRedundantInternalAccessibility(.struct("InternalStructConformingToExternalProtocol")) + // The protocol requirement implementation should NOT be flagged (line 10 in fixture) + assertNotRedundantInternalAccessibility(.functionMethodInstance("someExternalProtocolMethod()", line: 10)) + } + + /// Tests that implicitly internal types conforming to external protocols are NOT flagged. + func testImplicitlyInternalExternalProtocolConformanceNotFlagged() { + index() + + // Implicitly internal class conforming to ExternalProtocol should NOT be flagged + assertNotRedundantInternalAccessibility(.class("ImplicitlyInternalClassConformingToExternalProtocol")) + // The protocol requirement implementation should NOT be flagged (line 15 in fixture) + assertNotRedundantInternalAccessibility(.functionMethodInstance("someExternalProtocolMethod()", line: 15)) + } + + /// Tests that top-level internal types used only within the same file are flagged + /// as redundant internal. + /// + /// For top-level declarations, private and fileprivate are equivalent, so the + /// suggested accessibility is nil (ambiguous). Nested types are suppressed when + /// their parent is already flagged to reduce noise. + func testInternalTypeTransitivelyExposedInSameFileSuggestsFileprivate() { + index() + + // TransitiveExposureClassA is only used within its file (from ClassB), so it + // should be flagged as redundant internal. Since it's top-level, the suggestion + // is nil (private and fileprivate are equivalent for top-level declarations). + assertRedundantInternalAccessibility(.class("TransitiveExposureClassA")) + + // TransitivelyExposedStatus is suppressed because its parent (ClassA) is already + // flagged. This is by design to reduce noise - fixing the parent is sufficient. + } + + // MARK: - Transitive Access Exposure Tests + + // These tests verify that Periphery does NOT incorrectly flag internal types + // that are transitively exposed through API signatures when those APIs are + // called from other files. See TransitiveAccessExposure.swift for fixtures. + + /// Tests that internal types used in function/method signatures are NOT flagged + /// when the function is called from another file. + /// + /// Covers: parameter types, return types, default argument types, initializer parameters, + /// and closure types in properties. + func testTransitiveExposureThroughFunctionSignatures() { + index() + + // Parameter types: ParameterTypeA used in processParameter() + assertNotRedundantInternalAccessibility(.struct("ParameterTypeA")) + + // Return types: ReturnTypeA returned by getResult(), InternalReturnTypeEnum returned by getEnum() + assertNotRedundantInternalAccessibility(.enum("ReturnTypeA")) + assertNotRedundantInternalAccessibility(.enum("InternalReturnTypeEnum")) + + // Default argument types: DefaultArgTypeA used in processWithDefault() + assertNotRedundantInternalAccessibility(.struct("DefaultArgTypeA")) + + // Initializer parameters: InitParamTypeA used in init(config:) + assertNotRedundantInternalAccessibility(.struct("InitParamTypeA")) + + // Closure types: ClosureParamTypeA/ClosureReturnTypeA in transformer property + assertNotRedundantInternalAccessibility(.struct("ClosureParamTypeA")) + assertNotRedundantInternalAccessibility(.struct("ClosureReturnTypeA")) + } + + /// Tests that internal types used in property and subscript signatures are NOT flagged + /// when accessed from another file. + /// + /// Covers: property types, subscript parameter types, subscript return types. + func testTransitiveExposureThroughPropertyAndSubscriptSignatures() { + index() + + // Property types: PropertyTypeA used in exposedProperty + assertNotRedundantInternalAccessibility(.struct("PropertyTypeA")) + + // Subscript parameter types: SubscriptKeyTypeA used in subscript(key:) + assertNotRedundantInternalAccessibility(.struct("SubscriptKeyTypeA")) + + // Subscript return types: SubscriptReturnTypeA returned by subscript + assertNotRedundantInternalAccessibility(.struct("SubscriptReturnTypeA")) + } + + /// Tests that internal types used in generic constraints, protocol requirements, + /// enum associated values, and typealiases are NOT flagged when exposed from another file. + /// + /// Covers: generic constraint protocols, protocol requirement types, enum associated + /// value types, typealias target types. + func testTransitiveExposureThroughTypeSystemConstructs() { + index() + + // Generic constraints: GenericConstraintProtocolA constrains processGeneric() + assertNotRedundantInternalAccessibility(.protocol("GenericConstraintProtocolA")) + + // Protocol requirement types: ProtocolRequirementTypeA in ProtocolWithRequirementA + assertNotRedundantInternalAccessibility(.struct("ProtocolRequirementTypeA")) + + // Enum associated values: EnumAssociatedTypeA in EnumWithAssociatedValueA + assertNotRedundantInternalAccessibility(.struct("EnumAssociatedTypeA")) + + // Typealias targets: TypealiasTargetTypeA aliased by AliasedTypeA + assertNotRedundantInternalAccessibility(.struct("TypealiasTargetTypeA")) + } + + // MARK: - Enum Case Tests + + /// Tests that enum cases are NOT flagged as redundant internal. + /// + /// Enum cases cannot have explicit access modifiers in Swift - they always + /// inherit the accessibility of their containing enum. Suggesting to make + /// them private or fileprivate would cause a syntax error. + func testEnumCasesNotFlagged() { + index() + + // Enum cases should never be flagged + assertNotRedundantInternalAccessibility(.enumelement("usedCase")) + assertNotRedundantInternalAccessibility(.enumelement("anotherUsedCase")) + assertNotRedundantInternalAccessibility(.enumelement("internalCase")) + assertNotRedundantInternalAccessibility(.enumelement("anotherInternalCase")) + } + + // MARK: - Struct Memberwise Initializer Tests + + /// Tests that struct stored properties used in implicit memberwise initializers + /// are NOT flagged as redundant internal. + /// + /// Stored properties that are parameters to a struct's memberwise initializer + /// are part of the struct's public API. Even if they're only directly accessed + /// within the same file, they must remain accessible for the initializer. + func testStructMemberwiseInitPropertiesNotFlagged() { + index() + + // Properties used in memberwise init should not be flagged + assertNotRedundantInternalAccessibility(.varInstance("memberwiseProperty1")) + assertNotRedundantInternalAccessibility(.varInstance("memberwiseProperty2")) + assertNotRedundantInternalAccessibility(.varInstance("memberwiseConstant")) + assertNotRedundantInternalAccessibility(.varInstance("memberwiseProperty")) + assertNotRedundantInternalAccessibility(.varInstance("propertyWithDefault")) + } + + // MARK: - External Protocol Requirement Signature Tests + + /// Tests that types used in external protocol requirement signatures are NOT flagged. + /// + /// When a type is used as a return type or parameter type of a method that implements + /// an external protocol (like Equatable, NSViewRepresentable, etc.), the type must + /// remain internal because the protocol method's accessibility is constrained by the + /// protocol, and its signature types must be at least as accessible. + func testTypeUsedInExternalProtocolSignatureNotFlagged() { + index() + + // TypeUsedInExternalProtocolSignature conforms to Equatable, which means + // it's used as the parameter type for the == operator (an external protocol + // requirement). It should NOT be flagged as redundant internal. + assertNotRedundantInternalAccessibility(.struct("TypeUsedInExternalProtocolSignature")) + } + + // MARK: - Stored Property Type Transitive Exposure Tests + + /// Tests that types used as stored property types are NOT flagged when the containing + /// type is instantiated from another file. + /// + /// When a type T is used as a property type in struct/class C, and C is used from + /// outside the file, T is transitively exposed and must remain internal. + func testStoredPropertyTypeNotFlagged() { + index() + + // StoredPropertyRole is used as the type of StoredPropertyContainer.role + // StoredPropertyContainer is instantiated from StoredPropertyTypeExposure_Consumer.swift + assertNotRedundantInternalAccessibility(.enum("StoredPropertyRole")) + assertNotRedundantInternalAccessibility(.struct("StoredPropertyContainer")) + } + + /// Tests that nested types used as property types are NOT flagged when the containing + /// type is instantiated from another file. + func testNestedTypeAsPropertyTypeNotFlagged() { + index() + + // NestedPhase is used as the type of ClassWithNestedType.phase + // ClassWithNestedType is instantiated from StoredPropertyTypeExposure_Consumer.swift + assertNotRedundantInternalAccessibility(.enum("NestedPhase")) + assertNotRedundantInternalAccessibility(.class("ClassWithNestedType")) + } + + /// Tests that types in a chain of property types are NOT flagged when the outermost + /// type is instantiated from another file. + /// + /// This tests the recursive transitive exposure check: InnerType -> MiddleContainer -> OuterContainer + func testChainedPropertyTypeExposureNotFlagged() { + index() + + // InnerType is used in MiddleContainer, which is used in OuterContainer + // OuterContainer is instantiated from StoredPropertyTypeExposure_Consumer.swift + assertNotRedundantInternalAccessibility(.struct("InnerType")) + assertNotRedundantInternalAccessibility(.struct("MiddleContainer")) + assertNotRedundantInternalAccessibility(.struct("OuterContainer")) + } + + // MARK: - Same-File Type Constraint Tests + + /// Tests that types used as property types in internal types are NOT flagged + /// when the containing type must remain internal. + func testSameFilePropertyTypeConstraintNotFlagged() { + index() + + // SameFileConstrainedEnum is used as property type in SameFileConstrainingStruct + // SameFileConstrainingStruct is used from another file + assertNotRedundantInternalAccessibility(.enum("SameFileConstrainedEnum")) + assertNotRedundantInternalAccessibility(.struct("SameFileConstrainingStruct")) + } + + /// Tests that types used as return types are NOT flagged when the containing + /// type must remain internal. + func testSameFileReturnTypeConstraintNotFlagged() { + index() + + // SameFileReturnType is returned by SameFileClassWithReturnType.getReturnType() + // SameFileClassWithReturnType is used from another file + assertNotRedundantInternalAccessibility(.struct("SameFileReturnType")) + assertNotRedundantInternalAccessibility(.class("SameFileClassWithReturnType")) + } + + /// Tests that types used as parameter types are NOT flagged when the containing + /// type must remain internal. + func testSameFileParamTypeConstraintNotFlagged() { + index() + + // SameFileParamType is a parameter to SameFileClassWithParamType.process() + // SameFileClassWithParamType is used from another file + assertNotRedundantInternalAccessibility(.struct("SameFileParamType")) + assertNotRedundantInternalAccessibility(.class("SameFileClassWithParamType")) + } + + /// Tests that protocols used as generic constraints are NOT flagged when the + /// containing type must remain internal. + func testSameFileGenericConstraintNotFlagged() { + index() + + // SameFileConstraintProtocol is used as generic constraint in SameFileClassWithGenericConstraint + // SameFileClassWithGenericConstraint is used from another file + assertNotRedundantInternalAccessibility(.protocol("SameFileConstraintProtocol")) + assertNotRedundantInternalAccessibility(.class("SameFileClassWithGenericConstraint")) + } + + // MARK: - Memberwise Init Called From Different Type Tests + + /// Tests that struct properties are NOT flagged as redundant internal when + /// the struct's memberwise initializer is called from a different type in + /// the same file (e.g., a ViewModifier creating a struct). + func testMemberwiseInitCalledFromDifferentTypeNotFlagged() { + index() + + assertNotRedundantInternalAccessibility(.varInstance("crossTypeProperty1")) + assertNotRedundantInternalAccessibility(.varInstance("crossTypeProperty2")) + } + + // MARK: - Method Called From Free Function Tests + + /// Tests that type members are NOT flagged as private when called from + /// a free function in the same file. + /// + /// Free functions have no containing type, so members accessed from them + /// need at least fileprivate, not private. + func testMethodCalledFromFreeFunctionNotFlaggedAsPrivate() { + index() + + // Members called from a free function should suggest fileprivate, not private, + // because a free function has no containing type. + assertRedundantInternalAccessibility( + .functionMethodInstance("methodCalledFromFreeFunction()"), + suggestedAccessibility: .fileprivate + ) + assertRedundantInternalAccessibility( + .varInstance("propertyUsedFromFreeFunction"), + suggestedAccessibility: .fileprivate + ) + } + + // MARK: - Same-File Memberwise Init Tests + + /// Tests that struct memberwise init properties are NOT flagged when the struct + /// is instantiated in the same file AND the struct must remain internal due to + /// being used as a property type in another struct that's used from outside. + func testSameFileMemberwiseInitPropertiesNotFlagged() { + index() + + // SameFileMemberwiseStruct is used as property type in SameFileOuterStruct + // SameFileOuterStruct is used from another file + assertNotRedundantInternalAccessibility(.struct("SameFileMemberwiseStruct")) + assertNotRedundantInternalAccessibility(.struct("SameFileOuterStruct")) + } +} diff --git a/Tests/Shared/SourceGraphTestCase.swift b/Tests/Shared/SourceGraphTestCase.swift index 1177ed682..b713fb1a9 100644 --- a/Tests/Shared/SourceGraphTestCase.swift +++ b/Tests/Shared/SourceGraphTestCase.swift @@ -177,6 +177,71 @@ open class SourceGraphTestCase: XCTestCase { scopeStack.removeLast() } + func assertRedundantInternalAccessibility(_ description: DeclarationDescription, suggestedAccessibility: Accessibility? = nil, 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.results.redundantInternalAccessibilityDeclarations.contains(declaration) { + XCTFail("Expected declaration to have redundant internal accessibility: \(declaration)", file: file, line: line) + } + + if let suggestedAccessibility { + if let info = Self.graph.redundantInternalAccessibility[declaration] { + if info.suggestedAccessibility != suggestedAccessibility { + let actualText = info.suggestedAccessibility?.rawValue ?? "nil" + XCTFail("Expected suggested accessibility to be '\(suggestedAccessibility.rawValue)', but got '\(actualText)': \(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.results.redundantInternalAccessibilityDeclarations.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, containingTypeName: String? = nil, 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.results.redundantFilePrivateAccessibilityDeclarations.contains(declaration) { + XCTFail("Expected declaration to have redundant fileprivate accessibility: \(declaration)", file: file, line: line) + } + + if let containingTypeName { + if let info = Self.graph.redundantFilePrivateAccessibility[declaration] { + if info.containingTypeName != containingTypeName { + XCTFail("Expected containing type name to be '\(containingTypeName)', but got '\(info.containingTypeName ?? "nil")': \(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.results.redundantFilePrivateAccessibilityDeclarations.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) @@ -351,6 +416,26 @@ private extension [ScanResult] { } } + var redundantInternalAccessibilityDeclarations: Set { + compactMapSet { + if case .redundantInternalAccessibility = $0.annotation { + return $0.declaration + } + + return nil + } + } + + var redundantFilePrivateAccessibilityDeclarations: Set { + compactMapSet { + if case .redundantFilePrivateAccessibility = $0.annotation { + return $0.declaration + } + + return nil + } + } + var superfluousIgnoreCommandDeclarations: Set { compactMapSet { if case .superfluousIgnoreCommand = $0.annotation { diff --git a/baselines/bazel.json b/baselines/bazel.json index 835493c5d..80417a573 100644 --- a/baselines/bazel.json +++ b/baselines/bazel.json @@ -4,7 +4,11 @@ "s:11SourceGraph0aB8DebuggerC", "s:13SystemPackage8FilePathV10ExtensionsE5chdir7closureyyyKXE_tKF", "s:14SyntaxAnalysis21UnusedParameterParserV5parse4file0F9ProtocolsSayAA8FunctionVG11SourceGraph0J4FileC_SbtKFZ", - "s:14SyntaxAnalysis8FunctionV8fullNameSSvp" + "s:14SyntaxAnalysis8FunctionV8fullNameSSvp", + "s:14SyntaxAnalysis23UnusedParameterAnalyzerC7analyze8functionShyAA0D0VGAA8FunctionV_tF", + "s:14SyntaxAnalysis9ParameterV5labelAC8PartKindOvp", + "s:14SyntaxAnalysis9ParameterV8location11SourceGraph8LocationCvp", + "s:11SourceGraph32InterfaceBuilderPropertyRetainerC19swiftNameToSelectoryS2SFZ" ] } } diff --git a/baselines/linux-bazel.json b/baselines/linux-bazel.json index f55df1d7e..4aadd93bf 100644 --- a/baselines/linux-bazel.json +++ b/baselines/linux-bazel.json @@ -12,7 +12,11 @@ "s:6Shared17SetupGuideHelpersC6select8multipleAA0B9SelectionOSaySSG_tF", "s:SS10ExtensionsE17withEscapedQuotesSSvp", "s:SS10ExtensionsE4djb2Sivp", - "s:SS10ExtensionsE7djb2HexSSvp" + "s:SS10ExtensionsE7djb2HexSSvp", + "s:14SyntaxAnalysis23UnusedParameterAnalyzerC7analyze8functionShyAA0D0VGAA8FunctionV_tF", + "s:14SyntaxAnalysis9ParameterV5labelAC8PartKindOvp", + "s:14SyntaxAnalysis9ParameterV8location11SourceGraph8LocationCvp", + "s:11SourceGraph32InterfaceBuilderPropertyRetainerC19swiftNameToSelectoryS2SFZ" ] } }