Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add experimental unused import analysis. #682

Merged
merged 1 commit into from
Dec 11, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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: 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
26 changes: 21 additions & 5 deletions Sources/PeripheryKit/Syntax/ImportSyntaxVisitor.swift
Original file line number Diff line number Diff line change
@@ -1,16 +1,32 @@
import Foundation
import SwiftSyntax

final class ImportSyntaxVisitor: PeripherySyntaxVisitor {
typealias ImportStatement = (parts: [String], isTestable: Bool)
struct ImportStatement {
let module: String
let isTestable: Bool
let isExported: Bool
let location: SourceLocation
}

final class ImportSyntaxVisitor: PeripherySyntaxVisitor {
var importStatements: [ImportStatement] = []

init(sourceLocationBuilder: SourceLocationBuilder) {}
private let sourceLocationBuilder: SourceLocationBuilder

init(sourceLocationBuilder: SourceLocationBuilder) {
self.sourceLocationBuilder = sourceLocationBuilder
}

func visit(_ node: ImportDeclSyntax) {
let parts = node.path.map { $0.name.text }
let attributes = node.attributes.compactMap { $0.as(AttributeSyntax.self)?.attributeName.trimmedDescription }
importStatements.append((parts, attributes.contains("testable")))
let module = parts.first ?? ""
let attributes = node.attributes.compactMap { $0.as(AttributeSyntax.self)?.attributeName.trimmedDescription }
let location = sourceLocationBuilder.location(at: node.positionAfterSkippingLeadingTrivia)
let statement = ImportStatement(
module: module,
isTestable: attributes.contains("testable"),
isExported: attributes.contains("_exported"),
location: location)
importStatements.append(statement)
}
}
Loading
Loading