Skip to content

Commit

Permalink
Add experimental unused import analysis.
Browse files Browse the repository at this point in the history
  • Loading branch information
ileitch committed Dec 11, 2023
1 parent d24a727 commit 21e9ca5
Show file tree
Hide file tree
Showing 27 changed files with 217 additions and 36 deletions.
1 change: 1 addition & 0 deletions .periphery.linux.yml
Original file line number Diff line number Diff line change
Expand Up @@ -6,3 +6,4 @@ targets:
- PeripheryTests
- SPMTests
- AccessibilityTests
enable_unused_import_analysis: true
1 change: 1 addition & 0 deletions .periphery.yml
Original file line number Diff line number Diff line change
Expand Up @@ -8,3 +8,4 @@ targets:
- XcodeTests
- SPMTests
- AccessibilityTests
enable_unused_import_analysis: true
2 changes: 1 addition & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@

##### Enhancements

- None.
- Add experimental unused import analysis.

##### Bug Fixes

Expand Down
4 changes: 4 additions & 0 deletions Sources/Frontend/Commands/ScanCommand.swift
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,9 @@ struct ScanCommand: FrontendCommand {
@Flag(help: "Disable identification of redundant public accessibility")
var disableRedundantPublicAnalysis: Bool = defaultConfiguration.$disableRedundantPublicAnalysis.defaultValue

@Flag(help: "Enable identification of unused imports (experimental)")
var enableUnusedImportAnalysis: Bool = defaultConfiguration.$enableUnusedImportsAnalysis.defaultValue

@Flag(help: "Retain properties that are assigned, but never used")
var retainAssignOnlyProperties: Bool = defaultConfiguration.$retainAssignOnlyProperties.defaultValue

Expand Down Expand Up @@ -127,6 +130,7 @@ struct ScanCommand: FrontendCommand {
configuration.apply(\.$retainUnusedProtocolFuncParams, retainUnusedProtocolFuncParams)
configuration.apply(\.$retainSwiftUIPreviews, retainSwiftUIPreviews)
configuration.apply(\.$disableRedundantPublicAnalysis, disableRedundantPublicAnalysis)
configuration.apply(\.$enableUnusedImportsAnalysis, enableUnusedImportAnalysis)
configuration.apply(\.$externalEncodableProtocols, externalEncodableProtocols)
configuration.apply(\.$externalTestCaseClasses, externalTestCaseClasses)
configuration.apply(\.$verbose, verbose)
Expand Down
1 change: 0 additions & 1 deletion Sources/Frontend/main.swift
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import Foundation
import ArgumentParser
import PeripheryKit
import Shared

Logger.configureBuffering()
Expand Down
25 changes: 25 additions & 0 deletions Sources/PeripheryKit/Indexer/Declaration.swift
Original file line number Diff line number Diff line change
Expand Up @@ -109,10 +109,29 @@ final class Declaration {
}
}

var extensionKind: Kind? {
switch self {
case .class:
return .extensionClass
case .struct:
return .extensionStruct
case .enum:
return .extensionEnum
case .protocol:
return .extensionProtocol
default:
return nil
}
}

var isExtensionKind: Bool {
rawValue.hasPrefix("extension")
}

var isExtendableKind: Bool {
isConcreteTypeDeclarableKind
}

var isConformableKind: Bool {
isDiscreteConformableKind || isExtensionKind
}
Expand All @@ -125,6 +144,10 @@ final class Declaration {
return [.class, .struct, .enum]
}

var isConcreteTypeDeclarableKind: Bool {
Self.concreteTypeDeclarableKinds.contains(self)
}

static var concreteTypeDeclarableKinds: Set<Kind> {
return [.class, .struct, .enum, .typealias]
}
Expand All @@ -147,6 +170,8 @@ final class Declaration {

var displayName: String? {
switch self {
case .module:
return "imported module"
case .class:
return "class"
case .protocol:
Expand Down
2 changes: 0 additions & 2 deletions Sources/PeripheryKit/Indexer/SourceFile.swift
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,6 @@ import Foundation
import SystemPackage

class SourceFile {
typealias ImportStatement = (parts: [String], isTestable: Bool)

let path: FilePath
let modules: Set<String>
var importStatements: [ImportStatement] = []
Expand Down
12 changes: 12 additions & 0 deletions Sources/PeripheryKit/Indexer/SwiftIndexer.swift
Original file line number Diff line number Diff line change
Expand Up @@ -240,6 +240,14 @@ public final class SwiftIndexer: Indexer {
multiplexingSyntaxVisitor.visit()

sourceFile.importStatements = importSyntaxVisitor.importStatements

if configuration.enableUnusedImportsAnalysis {
for stmt in sourceFile.importStatements {
if stmt.isExported {
graph.addExportedModule(stmt.module, exportedBy: sourceFile.modules)
}
}
}

associateLatentReferences()
associateDanglingReferences()
Expand All @@ -266,6 +274,10 @@ public final class SwiftIndexer: Indexer {
}
}

if configuration.enableUnusedImportsAnalysis {
graph.addIndexedModules(modules)
}

let sourceFile = SourceFile(path: file, modules: modules)
self.sourceFile = sourceFile
return sourceFile
Expand Down
1 change: 0 additions & 1 deletion Sources/PeripheryKit/Indexer/XibParser.swift
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import Foundation
import AEXML
import SystemPackage
import Shared

final class XibParser {
private let path: FilePath
Expand Down
5 changes: 4 additions & 1 deletion Sources/PeripheryKit/SPM/SPMProjectDriver.swift
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,10 @@ extension SPMProjectDriver: ProjectDriver {
storePaths = [FilePath(package.path).appending(".build/debug/index/store")]
}

try SwiftIndexer(sourceFiles: sourceFiles, graph: graph, indexStorePaths: storePaths).perform()
try SwiftIndexer(
sourceFiles: sourceFiles,
graph: graph,
indexStorePaths: storePaths).perform()

graph.indexingComplete()
}
Expand Down
5 changes: 3 additions & 2 deletions Sources/PeripheryKit/ScanResultBuilder.swift
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
import Foundation
import Shared

public struct ScanResultBuilder {
public static func build(for graph: SourceGraph) -> [ScanResult] {
let assignOnlyProperties = graph.assignOnlyProperties
let removableDeclarations = graph.unusedDeclarations.subtracting(assignOnlyProperties)
let removableDeclarations = graph.unusedDeclarations
.subtracting(assignOnlyProperties)
.union(graph.unusedModuleImports)
let redundantProtocols = graph.redundantProtocols.filter { !removableDeclarations.contains($0.0) }
let redundantPublicAccessibility = graph.redundantPublicAccessibility.filter { !removableDeclarations.contains($0.0) }

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -159,8 +159,8 @@ final class RedundantExplicitPublicAccessibilityMarker: SourceGraphMutator {
let referenceFiles = graph.references(to: decl).map { $0.location.file }

let referenceModules = referenceFiles.flatMapSet { file -> Set<String> in
let importsDeclModuleTestable = file.importStatements.contains(where: { (parts, isTestable) in
isTestable && !Set(parts).isDisjoint(with: decl.location.file.modules)
let importsDeclModuleTestable = file.importStatements.contains(where: {
$0.isTestable && decl.location.file.modules.contains($0.module)
})

if !importsDeclModuleTestable {
Expand Down
82 changes: 82 additions & 0 deletions Sources/PeripheryKit/SourceGraph/Mutators/UnusedImportMarker.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,82 @@
import Foundation
import Shared

/// Marks unused import statements (experimental).
///
/// A module import is unused when the source file contains no references to it, and no other
/// imported modules either export it, or extend declarations declared by it.
///
/// Testing TODO:
/// * Exports, including nested exports
/// * Public declaration extended by another module
final class UnusedImportMarker: SourceGraphMutator {
private let graph: SourceGraph
private let configuration: Configuration

required init(graph: SourceGraph, configuration: Configuration) {
self.graph = graph
self.configuration = configuration
}

func mutate() throws {
guard configuration.enableUnusedImportsAnalysis else { return }

var referencedModulesByFile = [SourceFile: Set<String>]()

// Build a mapping of source files and the modules they reference.
for ref in graph.allReferences {
guard let decl = graph.explicitDeclaration(withUsr: ref.usr) else { continue }
// Record directly referenced modules and also identify any modules that extended
// the declaration. These extensions may provide members/conformances that aren't
// referenced directly but which are still required.
let referencedModules = decl.location.file.modules.union(modulesExtending(decl))
referencedModulesByFile[ref.location.file, default: []].formUnion(referencedModules)
}

// For each source file, determine whether its imports are unused.
for (file, referencedModules) in referencedModulesByFile {
let unreferencedImports = file.importStatements
.filter {
// Only consider modules that have been indexed as we need to see which modules
// they export.
graph.indexedModules.contains($0.module) &&
!referencedModules.contains($0.module)
}

for unreferencedImport in unreferencedImports {
// In the simple case, a module is unused if it's not referenced. However, it's
// possible the module exports other referenced modules.
guard !referencedModules.contains(where: {
graph.isModule($0, exportedBy: unreferencedImport.module)
}) else { continue }

graph.markUnusedModuleImport(unreferencedImport)
}
}
}

// MARK: - Private

private var extendedDeclCache: [Declaration: Set<String>] = [:]

/// Identifies any modules that extend the given declaration.
private func modulesExtending(_ decl: Declaration) -> Set<String> {
guard decl.kind.isExtendableKind else { return [] }

if let modules = extendedDeclCache[decl] {
return modules
}

let modules: Set<String> = graph.references(to: decl)
.flatMapSet {
guard let parent = $0.parent,
parent.kind == decl.kind.extensionKind,
parent.name == decl.name
else { return [] }

return parent.location.file.modules
}
extendedDeclCache[decl] = modules
return modules
}
}
55 changes: 47 additions & 8 deletions Sources/PeripheryKit/SourceGraph/SourceGraph.swift
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,13 @@ public final class SourceGraph {
private(set) var assetReferences: Set<AssetReference> = []
private(set) var mainAttributedDeclarations: Set<Declaration> = []
private(set) var allReferencesByUsr: [String: Set<Reference>] = [:]
private(set) var indexedModules: Set<String> = []
private(set) var unusedModuleImports: Set<Declaration> = []

private var potentialAssignOnlyProperties: Set<Declaration> = []
private var allDeclarationsByKind: [Declaration.Kind: Set<Declaration>] = [:]
private var allExplicitDeclarationsByUsr: [String: Declaration] = [:]
private var moduleToExportingModules: [String: Set<String>] = [:]

private let lock = UnfairLock()

Expand Down Expand Up @@ -150,12 +153,6 @@ public final class SourceGraph {
declaration.usrs.forEach { allExplicitDeclarationsByUsr.removeValue(forKey: $0) }
}

func add(_ reference: Reference) {
withLock {
addUnsafe(reference)
}
}

func addUnsafe(_ reference: Reference) {
_ = allReferences.insert(reference)
allReferencesByUsr[reference.usr, default: []].insert(reference)
Expand All @@ -173,9 +170,9 @@ public final class SourceGraph {
} else {
_ = declaration.references.insert(reference)
}
}

add(reference)
addUnsafe(reference)
}
}

func remove(_ reference: Reference) {
Expand Down Expand Up @@ -215,6 +212,48 @@ public final class SourceGraph {
explicitDeclaration(withUsr: reference.usr) == nil
}

func addIndexedModules(_ modules: Set<String>) {
withLock {
indexedModules.formUnion(modules)
}
}

func addExportedModule(_ module: String, exportedBy exportingModules: Set<String>) {
withLock {
moduleToExportingModules[module, default: []].formUnion(exportingModules)
}
}

func isModule(_ module: String, exportedBy exportingModule: String) -> Bool {
withLock {
isModuleUnsafe(module, exportedBy: exportingModule)
}
}

private func isModuleUnsafe(_ module: String, exportedBy exportingModule: String) -> Bool {
let exportingModules = moduleToExportingModules[module, default: []]

if exportingModules.contains(exportingModule) {
// The module is exported directly.
return true
}

// Recursively check if the module is exported transitively.
return exportingModules.contains { nestedExportingModule in
return isModuleUnsafe(nestedExportingModule, exportedBy: exportingModule) &&
isModuleUnsafe(module, exportedBy: nestedExportingModule)
}
}

func markUnusedModuleImport(_ statement: ImportStatement) {
withLock {
let usr = "\(statement.location.description)-\(statement.module)"
let decl = Declaration(kind: .module, usrs: [usr], location: statement.location)
decl.name = statement.module
unusedModuleImports.insert(decl)
}
}

func inheritedTypeReferences(of decl: Declaration, seenDeclarations: Set<Declaration> = []) -> [Reference] {
var references: [Reference] = []

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,9 @@ public final class SourceGraphMutatorRunner {
}

private let mutators: [SourceGraphMutator.Type] = [
// Must come before all others as we need to observe all references prior to any mutations.
UnusedImportMarker.self,

// Must come before ExtensionReferenceBuilder.
AccessibilityCascader.self,
ObjCAccessibleRetainer.self,
Expand Down Expand Up @@ -44,7 +47,7 @@ public final class SourceGraphMutatorRunner {
AssignOnlyPropertyReferenceEliminator.self,

UsedDeclarationMarker.self,
RedundantProtocolMarker.self
RedundantProtocolMarker.self,
]

private let graph: SourceGraph
Expand Down
Loading

0 comments on commit 21e9ca5

Please sign in to comment.