From fc2cd3b1e99182477906337decd3ac7a7cb98c2f Mon Sep 17 00:00:00 2001 From: Keith Smiley Date: Tue, 28 Mar 2023 11:39:58 -0700 Subject: [PATCH 01/20] Add unused-imports tool This tool uses the index data to suggest imports that can be removed from your files. Currently compared to SwiftLint's implementation it's much faster, but doesn't yet add missing imports, or remove unused system imports. It's currently lightly affected by upstream issues listed here https://github.com/lyft/swift-index-store/issues/5 --- Package.swift | 2 + Sources/IndexStore/IndexStore.swift | 6 +- Sources/unused-imports/main.swift | 195 ++++++++++++++++++++++++++++ 3 files changed, 200 insertions(+), 3 deletions(-) create mode 100644 Sources/unused-imports/main.swift diff --git a/Package.swift b/Package.swift index df7c874..ac21326 100644 --- a/Package.swift +++ b/Package.swift @@ -42,6 +42,7 @@ let package = Package( .library(name: "SwiftDemangle", targets: ["SwiftDemangle"]), .executable(name: "indexutil-export", targets: ["indexutil-export"]), .executable(name: "unnecessary-testable", targets: ["unnecessary-testable"]), + .executable(name: "unused-imports", targets: ["unused-imports"]), .executable(name: "indexutil-annotate", targets: ["indexutil-annotate"]), .executable(name: "tycat", targets: ["tycat"]), ], @@ -58,6 +59,7 @@ let package = Package( .testTarget(name: "SwiftDemangleTests", dependencies: ["SwiftDemangle"], exclude: ["BUILD"]), .executableTarget(name: "indexutil-export", dependencies: ["IndexStore"], exclude: ["BUILD"]), .executableTarget(name: "unnecessary-testable", dependencies: ["IndexStore"], exclude: ["BUILD"]), + .executableTarget(name: "unused-imports", dependencies: ["IndexStore"], exclude: ["BUILD"]), .executableTarget(name: "indexutil-annotate", dependencies: ["IndexStore"], exclude: ["BUILD"]), .executableTarget(name: "tycat", dependencies: ["IndexStore"], exclude: ["BUILD"]), ], diff --git a/Sources/IndexStore/IndexStore.swift b/Sources/IndexStore/IndexStore.swift index b5b6db4..472be99 100644 --- a/Sources/IndexStore/IndexStore.swift +++ b/Sources/IndexStore/IndexStore.swift @@ -94,7 +94,7 @@ public final class IndexStore { } } -public final class UnitReader: Sendable { +public final class UnitReader { private let reader: indexstore_unit_reader_t public init(indexStore: IndexStore, unitName: String) throws { @@ -130,9 +130,9 @@ public final class UnitReader: Sendable { return indexstore_unit_reader_is_debug_compilation(self.reader) } - public var mainFile: String { String(indexstore_unit_reader_get_main_file(self.reader)) } + public private(set) lazy var mainFile = String(indexstore_unit_reader_get_main_file(self.reader)) - public var moduleName: String { String(indexstore_unit_reader_get_module_name(self.reader)) } + public private(set) lazy var moduleName = String(indexstore_unit_reader_get_module_name(self.reader)) public var workingDirectory: String { String(indexstore_unit_reader_get_working_dir(self.reader)) } diff --git a/Sources/unused-imports/main.swift b/Sources/unused-imports/main.swift new file mode 100644 index 0000000..6b389b6 --- /dev/null +++ b/Sources/unused-imports/main.swift @@ -0,0 +1,195 @@ +import IndexStore +import Darwin +import Foundation + +// FIXME: Loosen this regex +// FIXME: this ignores 'import foo.bar' and all other 'import struct foo.bar' things +private let testableRegex = try NSRegularExpression( + pattern: #"^[^/\n]*\bimport ([^ \n.]+)( *// *noqa)?$"#, options: [.anchorsMatchLines]) +// FIXME: This isn't complete +private let identifierRegex = try NSRegularExpression( + pattern: "([a-zA-Z_][a-zA-Z0-9_]*)", options: []) + +private func getImports(path: String) -> Set { + guard let searchText = try? String(contentsOfFile: path) else { + fatalError("failed to read '\(path)'") + } + + let matches = testableRegex.matches( + in: searchText, range: NSRange(searchText.startIndex.. Storage { + // Empty source files have units but no records + guard let recordReader else { + return Storage(usrs: [], typealiases: []) + } + + var lines: [String.SubSequence]? + var usrs = Set() + var typealiasExts = Set() + recordReader.forEach { (occurrence: SymbolOccurrence) in + if occurrence.symbol.subkind == .swiftExtensionOfStruct { + usrs.insert(occurrence.symbol.usr) + if lines == nil { + lines = try! String(contentsOfFile: unitReader.mainFile).split(separator: "\n", omittingEmptySubsequences: false) + } + + let line = String(lines![occurrence.location.line - 1]) + let indexes = line.index(line.startIndex, offsetBy: occurrence.location.column - 1).. ([UnitReader], [String: RecordReader]) { + let store: IndexStore + do { + store = try IndexStore(path: indexStorePath) + } catch { + fatalError("error: failed to open index store: \(error)") + } + + var units: [UnitReader] = [] + var unitToRecord: [String: RecordReader] = [:] + + for unitReader in store.units { + if unitReader.mainFile.isEmpty { + continue + } + + units.append(unitReader) + if let recordName = unitReader.recordName { + let recordReader: RecordReader + do { + recordReader = try RecordReader(indexStore: store, recordName: recordName) + } catch { + fatalError("error: failed to load record: \(recordName) \(error)") + } + + // FIXME: Duplicates can happen if a single file is included in multiple targets / configurations + // if let existingRecord = unitToRecord[unitReader.mainFile] { + // // fatalError("error: found duplicate record for \(unitReader.mainFile) in \(existingRecord.name) and \(recordReader.name)") + // } + + unitToRecord[unitReader.mainFile] = recordReader + } + } + + if units.isEmpty { + fatalError("error: failed to load units from \(indexStorePath)") + } + + return (units, unitToRecord) +} + +struct Storage { + let usrs: Set + let typealiases: Set +} + +func main(indexStorePath: String) { + if let directory = ProcessInfo.processInfo.environment["BUILD_WORKSPACE_DIRECTORY"] { + FileManager.default.changeCurrentDirectoryPath(directory) + } + + var filesToUSRDefinitions: [String: Storage] = [:] + let (units, unitToRecord) = collectUnitsAndRecords(indexStorePath: indexStorePath) + var modulesToUnits: [String: [UnitReader]] = [:] + var allModuleNames = Set() + for unitReader in units { + allModuleNames.insert(unitReader.moduleName) + modulesToUnits[unitReader.moduleName, default: []].append(unitReader) + + if let recordReader = unitToRecord[unitReader.mainFile] { + var definedUsrs = Set() + var definedTypealiases = Set() + + recordReader.forEach { (occurrence: SymbolOccurrence) in + if occurrence.roles.contains(.definition) { + definedUsrs.insert(occurrence.symbol.usr) + if occurrence.symbol.kind == .typealias { + definedTypealiases.insert(occurrence.symbol.name) + } + } + + } + + filesToUSRDefinitions[unitReader.mainFile] = Storage( + usrs: definedUsrs, typealiases: definedTypealiases) + } + } + + for unitReader in units { + let allImports = getImports(path: unitReader.mainFile).intersection(allModuleNames) + if allImports.isEmpty { + continue + } + + let referencedUSRs = getReferenceUSRs(unitReader: unitReader, recordReader: unitToRecord[unitReader.mainFile]) + var usedImports = Set() + for anImport in allImports { + for dependentUnit in modulesToUnits[anImport] ?? [] { + if usedImports.contains(anImport) { + break + } + + // Empty files have units but no records and therefore no usrs + guard let storage = filesToUSRDefinitions[dependentUnit.mainFile] else { + continue + } + + + if !storage.usrs.intersection(referencedUSRs.usrs).isEmpty { + usedImports.insert(dependentUnit.moduleName) + } + + if !storage.typealiases.intersection(referencedUSRs.typealiases).isEmpty { + // If the type alias isn't already imported then it's probably not the one we're looking for + if allImports.contains(dependentUnit.moduleName) { + usedImports.insert(dependentUnit.moduleName) + } + } + } + + if allImports.subtracting(usedImports).isEmpty { + break + } + } + + for module in allImports.intersection(allModuleNames).subtracting(usedImports) { + print("/usr/bin/sed -i \"\" '/^import \(module)$/d' \(unitReader.mainFile)") + } + } +} + +main(indexStorePath: CommandLine.arguments[1]) From dcae0958e6a5aed0614dc6a5c5bd7202ca3c7eb4 Mon Sep 17 00:00:00 2001 From: Keith Smiley Date: Tue, 28 Mar 2023 12:48:22 -0700 Subject: [PATCH 02/20] add bazel config --- Sources/unused-imports/BUILD | 11 +++++++++++ 1 file changed, 11 insertions(+) create mode 100644 Sources/unused-imports/BUILD diff --git a/Sources/unused-imports/BUILD b/Sources/unused-imports/BUILD new file mode 100644 index 0000000..e199cd8 --- /dev/null +++ b/Sources/unused-imports/BUILD @@ -0,0 +1,11 @@ +load("@build_bazel_rules_swift//swift:swift.bzl", "swift_binary") + +swift_binary( + name = "unused-imports", + srcs = [ + "main.swift", + ], + deps = [ + "//:IndexStore", + ], +) From 45d95897fce9702004a54617020bcb9036b38900 Mon Sep 17 00:00:00 2001 From: Keith Smiley Date: Tue, 28 Mar 2023 14:34:47 -0700 Subject: [PATCH 03/20] handle filtering --- Sources/unused-imports/main.swift | 29 +++++++++++++++++++++++++++-- 1 file changed, 27 insertions(+), 2 deletions(-) diff --git a/Sources/unused-imports/main.swift b/Sources/unused-imports/main.swift index 6b389b6..0fd04cf 100644 --- a/Sources/unused-imports/main.swift +++ b/Sources/unused-imports/main.swift @@ -117,7 +117,11 @@ struct Storage { let typealiases: Set } -func main(indexStorePath: String) { +func main( + indexStorePath: String, + ignoredFileRegex: Regex?, + ignoredModuleRegex: Regex?) +{ if let directory = ProcessInfo.processInfo.environment["BUILD_WORKSPACE_DIRECTORY"] { FileManager.default.changeCurrentDirectoryPath(directory) } @@ -150,6 +154,12 @@ func main(indexStorePath: String) { } for unitReader in units { + if (try? ignoredFileRegex?.wholeMatch(in: unitReader.mainFile)) != nil { + continue + } else if (try? ignoredModuleRegex?.wholeMatch(in: unitReader.moduleName)) != nil { + continue + } + let allImports = getImports(path: unitReader.mainFile).intersection(allModuleNames) if allImports.isEmpty { continue @@ -192,4 +202,19 @@ func main(indexStorePath: String) { } } -main(indexStorePath: CommandLine.arguments[1]) +if CommandLine.arguments.count == 4 { + let ignoredFileRegex = try! Regex(CommandLine.arguments[2]) + let ignoredModuleRegex = try! Regex(CommandLine.arguments[3]) + + main( + indexStorePath: CommandLine.arguments[1], + ignoredFileRegex: ignoredFileRegex, + ignoredModuleRegex: ignoredModuleRegex + ) +} else { + main( + indexStorePath: CommandLine.arguments[1], + ignoredFileRegex: nil, + ignoredModuleRegex: nil + ) +} From 7cb6ca73c2527d1ef2e5ef0420a36eb98fa65cb0 Mon Sep 17 00:00:00 2001 From: Keith Smiley Date: Tue, 28 Mar 2023 15:05:06 -0700 Subject: [PATCH 04/20] lol ergonomics --- Sources/unused-imports/main.swift | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Sources/unused-imports/main.swift b/Sources/unused-imports/main.swift index 0fd04cf..0260fde 100644 --- a/Sources/unused-imports/main.swift +++ b/Sources/unused-imports/main.swift @@ -154,9 +154,9 @@ func main( } for unitReader in units { - if (try? ignoredFileRegex?.wholeMatch(in: unitReader.mainFile)) != nil { + if let ignoredFileRegex, unitReader.mainFile.wholeMatch(of: ignoredFileRegex) != nil { continue - } else if (try? ignoredModuleRegex?.wholeMatch(in: unitReader.moduleName)) != nil { + } else if let ignoredModuleRegex, unitReader.moduleName.wholeMatch(of: ignoredModuleRegex) != nil { continue } From 01bfc5057d26d223e85e1897f906da16c31fd683 Mon Sep 17 00:00:00 2001 From: Keith Smiley Date: Tue, 28 Mar 2023 17:19:07 -0700 Subject: [PATCH 05/20] Other discover option --- Sources/unused-imports/main.swift | 44 ++++++++++++++++--------------- 1 file changed, 23 insertions(+), 21 deletions(-) diff --git a/Sources/unused-imports/main.swift b/Sources/unused-imports/main.swift index 0260fde..4757fe0 100644 --- a/Sources/unused-imports/main.swift +++ b/Sources/unused-imports/main.swift @@ -9,30 +9,30 @@ private let testableRegex = try NSRegularExpression( // FIXME: This isn't complete private let identifierRegex = try NSRegularExpression( pattern: "([a-zA-Z_][a-zA-Z0-9_]*)", options: []) +private let importRegex = try Regex(#"\bimport\b"#) +private let noqaRegex = try Regex(#"// *noqa"#) -private func getImports(path: String) -> Set { - guard let searchText = try? String(contentsOfFile: path) else { - fatalError("failed to read '\(path)'") +private func getImports(path: String, recordReader: RecordReader?) -> (Set, [String: Int]) { + guard let recordReader else { + return ([], [:]) } - let matches = testableRegex.matches( - in: searchText, range: NSRange(searchText.startIndex..() + recordReader.forEach { (occurrence: SymbolOccurrence) in + if occurrence.symbol.kind == .module && occurrence.roles.contains(.reference) { + let line = lines[occurrence.location.line - 1] + // FIXME: This won't work if we are also adding missing imports, return it separately + if line.firstMatch(of: importRegex) != nil && line.firstMatch(of: noqaRegex) == nil { + imports.insert(occurrence.symbol.name) + importsToLineNumbers[occurrence.symbol.name] = occurrence.location.line } } + } - return String(searchText[range]) - }) + return (imports, importsToLineNumbers) } private func getReferenceUSRs(unitReader: UnitReader, recordReader: RecordReader?) -> Storage { @@ -160,7 +160,8 @@ func main( continue } - let allImports = getImports(path: unitReader.mainFile).intersection(allModuleNames) + let (rawImports, importsToLineNumbers) = getImports(path: unitReader.mainFile, recordReader: unitToRecord[unitReader.mainFile]) + let allImports = rawImports.intersection(allModuleNames) if allImports.isEmpty { continue } @@ -178,7 +179,6 @@ func main( continue } - if !storage.usrs.intersection(referencedUSRs.usrs).isEmpty { usedImports.insert(dependentUnit.moduleName) } @@ -196,8 +196,10 @@ func main( } } - for module in allImports.intersection(allModuleNames).subtracting(usedImports) { - print("/usr/bin/sed -i \"\" '/^import \(module)$/d' \(unitReader.mainFile)") + let unusedImports = allImports.intersection(allModuleNames).subtracting(usedImports) + if !unusedImports.isEmpty { + let sedCmd = unusedImports.map { "\(importsToLineNumbers[$0]!)d" }.sorted().joined(separator: ";") + print("/usr/bin/sed -i \"\" '\(sedCmd)' \(unitReader.mainFile)") } } } From e288dc91c21b86ad6039e1c2f7d5c296df6f8a4a Mon Sep 17 00:00:00 2001 From: Keith Smiley Date: Wed, 29 Mar 2023 10:21:29 -0700 Subject: [PATCH 06/20] small cleanups --- Sources/unused-imports/main.swift | 33 +++++++++++-------------------- 1 file changed, 11 insertions(+), 22 deletions(-) diff --git a/Sources/unused-imports/main.swift b/Sources/unused-imports/main.swift index 4757fe0..bd91ae8 100644 --- a/Sources/unused-imports/main.swift +++ b/Sources/unused-imports/main.swift @@ -2,10 +2,6 @@ import IndexStore import Darwin import Foundation -// FIXME: Loosen this regex -// FIXME: this ignores 'import foo.bar' and all other 'import struct foo.bar' things -private let testableRegex = try NSRegularExpression( - pattern: #"^[^/\n]*\bimport ([^ \n.]+)( *// *noqa)?$"#, options: [.anchorsMatchLines]) // FIXME: This isn't complete private let identifierRegex = try NSRegularExpression( pattern: "([a-zA-Z_][a-zA-Z0-9_]*)", options: []) @@ -89,19 +85,12 @@ private func collectUnitsAndRecords(indexStorePath: String) -> ([UnitReader], [S units.append(unitReader) if let recordName = unitReader.recordName { - let recordReader: RecordReader do { - recordReader = try RecordReader(indexStore: store, recordName: recordName) + let recordReader = try RecordReader(indexStore: store, recordName: recordName) + unitToRecord[unitReader.mainFile] = recordReader } catch { fatalError("error: failed to load record: \(recordName) \(error)") } - - // FIXME: Duplicates can happen if a single file is included in multiple targets / configurations - // if let existingRecord = unitToRecord[unitReader.mainFile] { - // // fatalError("error: found duplicate record for \(unitReader.mainFile) in \(existingRecord.name) and \(recordReader.name)") - // } - - unitToRecord[unitReader.mainFile] = recordReader } } @@ -160,13 +149,15 @@ func main( continue } - let (rawImports, importsToLineNumbers) = getImports(path: unitReader.mainFile, recordReader: unitToRecord[unitReader.mainFile]) + let (rawImports, importsToLineNumbers) = getImports( + path: unitReader.mainFile, recordReader: unitToRecord[unitReader.mainFile]) let allImports = rawImports.intersection(allModuleNames) if allImports.isEmpty { continue } - let referencedUSRs = getReferenceUSRs(unitReader: unitReader, recordReader: unitToRecord[unitReader.mainFile]) + let referencedUSRs = getReferenceUSRs( + unitReader: unitReader, recordReader: unitToRecord[unitReader.mainFile]) var usedImports = Set() for anImport in allImports { for dependentUnit in modulesToUnits[anImport] ?? [] { @@ -181,10 +172,8 @@ func main( if !storage.usrs.intersection(referencedUSRs.usrs).isEmpty { usedImports.insert(dependentUnit.moduleName) - } - - if !storage.typealiases.intersection(referencedUSRs.typealiases).isEmpty { - // If the type alias isn't already imported then it's probably not the one we're looking for + } else if !storage.typealiases.intersection(referencedUSRs.typealiases).isEmpty { + // If the typealias isn't already imported then it's probably not the one we're looking for if allImports.contains(dependentUnit.moduleName) { usedImports.insert(dependentUnit.moduleName) } @@ -196,7 +185,7 @@ func main( } } - let unusedImports = allImports.intersection(allModuleNames).subtracting(usedImports) + let unusedImports = allImports.subtracting(usedImports) if !unusedImports.isEmpty { let sedCmd = unusedImports.map { "\(importsToLineNumbers[$0]!)d" }.sorted().joined(separator: ";") print("/usr/bin/sed -i \"\" '\(sedCmd)' \(unitReader.mainFile)") @@ -205,8 +194,8 @@ func main( } if CommandLine.arguments.count == 4 { - let ignoredFileRegex = try! Regex(CommandLine.arguments[2]) - let ignoredModuleRegex = try! Regex(CommandLine.arguments[3]) + let ignoredFileRegex = try Regex(CommandLine.arguments[2]) + let ignoredModuleRegex = try Regex(CommandLine.arguments[3]) main( indexStorePath: CommandLine.arguments[1], From 77c04c57ac505b2cf8701dc7846b4a88b427aa8a Mon Sep 17 00:00:00 2001 From: Keith Smiley Date: Wed, 29 Mar 2023 15:22:18 -0700 Subject: [PATCH 07/20] relative paths, new syntax --- Sources/unused-imports/main.swift | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/Sources/unused-imports/main.swift b/Sources/unused-imports/main.swift index bd91ae8..3e8e21d 100644 --- a/Sources/unused-imports/main.swift +++ b/Sources/unused-imports/main.swift @@ -6,7 +6,7 @@ import Foundation private let identifierRegex = try NSRegularExpression( pattern: "([a-zA-Z_][a-zA-Z0-9_]*)", options: []) private let importRegex = try Regex(#"\bimport\b"#) -private let noqaRegex = try Regex(#"// *noqa"#) +private let ignoreRegex = try Regex(#"// *ignore-import"#) private func getImports(path: String, recordReader: RecordReader?) -> (Set, [String: Int]) { guard let recordReader else { @@ -21,7 +21,7 @@ private func getImports(path: String, recordReader: RecordReader?) -> (Set Date: Wed, 29 Mar 2023 15:41:07 -0700 Subject: [PATCH 08/20] update macos version --- .bazelrc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.bazelrc b/.bazelrc index 3752ba0..198caa8 100644 --- a/.bazelrc +++ b/.bazelrc @@ -1,3 +1,3 @@ build --incompatible_disallow_empty_glob -build --macos_minimum_os=12.0 -build --host_macos_minimum_os=12.0 +build --macos_minimum_os=13.0 +build --host_macos_minimum_os=13.0 From 7ca89fa5224e6c639295aa041b04b3fc3f78b261 Mon Sep 17 00:00:00 2001 From: Keith Smiley Date: Thu, 30 Mar 2023 17:29:10 -0700 Subject: [PATCH 09/20] improve sort --- Sources/unused-imports/main.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Sources/unused-imports/main.swift b/Sources/unused-imports/main.swift index 3e8e21d..a31d113 100644 --- a/Sources/unused-imports/main.swift +++ b/Sources/unused-imports/main.swift @@ -188,7 +188,7 @@ func main( let unusedImports = allImports.subtracting(usedImports) if !unusedImports.isEmpty { - let sedCmd = unusedImports.map { "\(importsToLineNumbers[$0]!)d" }.sorted().joined(separator: ";") + let sedCmd = unusedImports.map { importsToLineNumbers[$0]! }.sorted().map { "\($0)d" }.joined(separator: ";") let relativePath = unitReader.mainFile.replacingOccurrences(of: pwd + "/", with: "") print("/usr/bin/sed -i \"\" '\(sedCmd)' \(relativePath)") } From f0750bed9eb9fdb458897ae0a9fa9b6f0a2d2e65 Mon Sep 17 00:00:00 2001 From: Keith Smiley Date: Fri, 31 Mar 2023 09:42:31 -0700 Subject: [PATCH 10/20] minor perf --- Sources/unused-imports/main.swift | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/Sources/unused-imports/main.swift b/Sources/unused-imports/main.swift index a31d113..2c5b2f8 100644 --- a/Sources/unused-imports/main.swift +++ b/Sources/unused-imports/main.swift @@ -5,8 +5,8 @@ import Foundation // FIXME: This isn't complete private let identifierRegex = try NSRegularExpression( pattern: "([a-zA-Z_][a-zA-Z0-9_]*)", options: []) -private let importRegex = try Regex(#"\bimport\b"#) -private let ignoreRegex = try Regex(#"// *ignore-import"#) +private let importRegex = try Regex(#"\bimport "#) +private let ignoreRegex = try Regex(#"// *ignore-import$"#) private func getImports(path: String, recordReader: RecordReader?) -> (Set, [String: Int]) { guard let recordReader else { @@ -21,7 +21,9 @@ private func getImports(path: String, recordReader: RecordReader?) -> (Set Date: Fri, 31 Mar 2023 10:56:54 -0700 Subject: [PATCH 11/20] perf optimizations --- Sources/unused-imports/main.swift | 19 +++++++------------ 1 file changed, 7 insertions(+), 12 deletions(-) diff --git a/Sources/unused-imports/main.swift b/Sources/unused-imports/main.swift index 2c5b2f8..50eda78 100644 --- a/Sources/unused-imports/main.swift +++ b/Sources/unused-imports/main.swift @@ -5,9 +5,10 @@ import Foundation // FIXME: This isn't complete private let identifierRegex = try NSRegularExpression( pattern: "([a-zA-Z_][a-zA-Z0-9_]*)", options: []) -private let importRegex = try Regex(#"\bimport "#) private let ignoreRegex = try Regex(#"// *ignore-import$"#) +private var cachedLines = [String: [String.SubSequence]]() + private func getImports(path: String, recordReader: RecordReader?) -> (Set, [String: Int]) { guard let recordReader else { return ([], [:]) @@ -15,13 +16,14 @@ private func getImports(path: String, recordReader: RecordReader?) -> (Set() recordReader.forEach { (occurrence: SymbolOccurrence) in if occurrence.symbol.kind == .module && occurrence.roles.contains(.reference) { let line = lines[occurrence.location.line - 1] // FIXME: This won't work if we are also adding missing imports, return it separately - if (line.hasPrefix("import ") || line.firstMatch(of: importRegex) != nil) && + if (line.hasPrefix("import ") || line.contains(" import ")) && line.firstMatch(of: ignoreRegex) == nil { imports.insert(occurrence.symbol.name) @@ -39,17 +41,13 @@ private func getReferenceUSRs(unitReader: UnitReader, recordReader: RecordReader return Storage(usrs: [], typealiases: []) } - var lines: [String.SubSequence]? var usrs = Set() var typealiasExts = Set() recordReader.forEach { (occurrence: SymbolOccurrence) in if occurrence.symbol.subkind == .swiftExtensionOfStruct { usrs.insert(occurrence.symbol.usr) - if lines == nil { - lines = try! String(contentsOfFile: unitReader.mainFile).split(separator: "\n", omittingEmptySubsequences: false) - } - - let line = String(lines![occurrence.location.line - 1]) + let lines = cachedLines[unitReader.mainFile]! + let line = String(lines[occurrence.location.line - 1]) let indexes = line.index(line.startIndex, offsetBy: occurrence.location.column - 1).. ([UnitReader], [S return (units, unitToRecord) } -struct Storage { - let usrs: Set - let typealiases: Set -} +typealias Storage = (usrs: Set, typealiases: Set) func main( indexStorePath: String, From d648deaafd8e9cc875fcf1318a9e80cea4c46470 Mon Sep 17 00:00:00 2001 From: Keith Smiley Date: Fri, 31 Mar 2023 11:17:50 -0700 Subject: [PATCH 12/20] improve naming --- Sources/unused-imports/main.swift | 26 ++++++++++++++------------ 1 file changed, 14 insertions(+), 12 deletions(-) diff --git a/Sources/unused-imports/main.swift b/Sources/unused-imports/main.swift index 50eda78..39f0c3e 100644 --- a/Sources/unused-imports/main.swift +++ b/Sources/unused-imports/main.swift @@ -2,11 +2,11 @@ import IndexStore import Darwin import Foundation +private typealias References = (usrs: Set, typealiases: Set) // FIXME: This isn't complete private let identifierRegex = try NSRegularExpression( pattern: "([a-zA-Z_][a-zA-Z0-9_]*)", options: []) private let ignoreRegex = try Regex(#"// *ignore-import$"#) - private var cachedLines = [String: [String.SubSequence]]() private func getImports(path: String, recordReader: RecordReader?) -> (Set, [String: Int]) { @@ -35,10 +35,10 @@ private func getImports(path: String, recordReader: RecordReader?) -> (Set Storage { +private func getReferences(unitReader: UnitReader, recordReader: RecordReader?) -> References { // Empty source files have units but no records guard let recordReader else { - return Storage(usrs: [], typealiases: []) + return References(usrs: [], typealiases: []) } var usrs = Set() @@ -64,7 +64,7 @@ private func getReferenceUSRs(unitReader: UnitReader, recordReader: RecordReader } } - return Storage(usrs: usrs, typealiases: typealiasExts) + return References(usrs: usrs, typealiases: typealiasExts) } private func collectUnitsAndRecords(indexStorePath: String) -> ([UnitReader], [String: RecordReader]) { @@ -101,8 +101,6 @@ private func collectUnitsAndRecords(indexStorePath: String) -> ([UnitReader], [S return (units, unitToRecord) } -typealias Storage = (usrs: Set, typealiases: Set) - func main( indexStorePath: String, ignoredFileRegex: Regex?, @@ -113,10 +111,12 @@ func main( } let pwd = FileManager.default.currentDirectoryPath - var filesToUSRDefinitions: [String: Storage] = [:] + var filesToDefinitions: [String: References] = [:] let (units, unitToRecord) = collectUnitsAndRecords(indexStorePath: indexStorePath) var modulesToUnits: [String: [UnitReader]] = [:] var allModuleNames = Set() + var allDefinitionUsrs = Set() + for unitReader in units { allModuleNames.insert(unitReader.moduleName) modulesToUnits[unitReader.moduleName, default: []].append(unitReader) @@ -128,6 +128,8 @@ func main( recordReader.forEach { (occurrence: SymbolOccurrence) in if occurrence.roles.contains(.definition) { definedUsrs.insert(occurrence.symbol.usr) + allDefinitionUsrs.insert(occurrence.symbol.usr) + if occurrence.symbol.kind == .typealias { definedTypealiases.insert(occurrence.symbol.name) } @@ -135,7 +137,7 @@ func main( } - filesToUSRDefinitions[unitReader.mainFile] = Storage( + filesToDefinitions[unitReader.mainFile] = References( usrs: definedUsrs, typealiases: definedTypealiases) } } @@ -154,7 +156,7 @@ func main( continue } - let referencedUSRs = getReferenceUSRs( + let references = getReferences( unitReader: unitReader, recordReader: unitToRecord[unitReader.mainFile]) var usedImports = Set() for anImport in allImports { @@ -164,13 +166,13 @@ func main( } // Empty files have units but no records and therefore no usrs - guard let storage = filesToUSRDefinitions[dependentUnit.mainFile] else { + guard let definitions = filesToDefinitions[dependentUnit.mainFile] else { continue } - if !storage.usrs.intersection(referencedUSRs.usrs).isEmpty { + if !definitions.usrs.intersection(references.usrs).isEmpty { usedImports.insert(dependentUnit.moduleName) - } else if !storage.typealiases.intersection(referencedUSRs.typealiases).isEmpty { + } else if !definitions.typealiases.intersection(references.typealiases).isEmpty { // If the typealias isn't already imported then it's probably not the one we're looking for if allImports.contains(dependentUnit.moduleName) { usedImports.insert(dependentUnit.moduleName) From 99fb667ee0f8ea1f1e32d8e1ff23835169f172b3 Mon Sep 17 00:00:00 2001 From: Keith Smiley Date: Fri, 31 Mar 2023 13:41:01 -0700 Subject: [PATCH 13/20] fix duplicate units --- Sources/unused-imports/main.swift | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/Sources/unused-imports/main.swift b/Sources/unused-imports/main.swift index 39f0c3e..ce3edca 100644 --- a/Sources/unused-imports/main.swift +++ b/Sources/unused-imports/main.swift @@ -83,6 +83,12 @@ private func collectUnitsAndRecords(indexStorePath: String) -> ([UnitReader], [S continue } + // If files are built in multiple configurations, we just pick the first one. + // Otherwise it's unclear which module(s) things are actually a part of. + if unitToRecord[unitReader.mainFile] != nil { + continue + } + units.append(unitReader) if let recordName = unitReader.recordName { do { From 76cdf12cf33431ff1301c3a5264dc9a2f2254c0e Mon Sep 17 00:00:00 2001 From: Keith Smiley Date: Fri, 31 Mar 2023 14:01:24 -0700 Subject: [PATCH 14/20] CI --- .github/workflows/bazel.yml | 1 + Sources/unused-imports/BUILD | 3 +++ 2 files changed, 4 insertions(+) diff --git a/.github/workflows/bazel.yml b/.github/workflows/bazel.yml index c9dac61..d67b3ee 100644 --- a/.github/workflows/bazel.yml +++ b/.github/workflows/bazel.yml @@ -10,3 +10,4 @@ jobs: - uses: actions/checkout@v3 - uses: swift-actions/setup-swift@v1 - run: bazelisk test //Tests/... --test_output=errors + - run: swift test diff --git a/Sources/unused-imports/BUILD b/Sources/unused-imports/BUILD index e199cd8..96716e3 100644 --- a/Sources/unused-imports/BUILD +++ b/Sources/unused-imports/BUILD @@ -5,6 +5,9 @@ swift_binary( srcs = [ "main.swift", ], + tags = [ + "manual", + ], deps = [ "//:IndexStore", ], From 6c9956f11b85e1f3ba804bce92024fe45004a2e8 Mon Sep 17 00:00:00 2001 From: Keith Smiley Date: Fri, 31 Mar 2023 14:02:07 -0700 Subject: [PATCH 15/20] unused var --- Sources/unused-imports/main.swift | 2 -- 1 file changed, 2 deletions(-) diff --git a/Sources/unused-imports/main.swift b/Sources/unused-imports/main.swift index ce3edca..81296c3 100644 --- a/Sources/unused-imports/main.swift +++ b/Sources/unused-imports/main.swift @@ -121,7 +121,6 @@ func main( let (units, unitToRecord) = collectUnitsAndRecords(indexStorePath: indexStorePath) var modulesToUnits: [String: [UnitReader]] = [:] var allModuleNames = Set() - var allDefinitionUsrs = Set() for unitReader in units { allModuleNames.insert(unitReader.moduleName) @@ -134,7 +133,6 @@ func main( recordReader.forEach { (occurrence: SymbolOccurrence) in if occurrence.roles.contains(.definition) { definedUsrs.insert(occurrence.symbol.usr) - allDefinitionUsrs.insert(occurrence.symbol.usr) if occurrence.symbol.kind == .typealias { definedTypealiases.insert(occurrence.symbol.name) From 2fec10133deccea1ef1865c4d7eae362f72a91d8 Mon Sep 17 00:00:00 2001 From: Keith Smiley Date: Fri, 31 Mar 2023 14:13:20 -0700 Subject: [PATCH 16/20] remove unnecessary dictionary perf win --- Sources/unused-imports/main.swift | 65 ++++++++++++------------------- 1 file changed, 25 insertions(+), 40 deletions(-) diff --git a/Sources/unused-imports/main.swift b/Sources/unused-imports/main.swift index 81296c3..8ae54d7 100644 --- a/Sources/unused-imports/main.swift +++ b/Sources/unused-imports/main.swift @@ -9,11 +9,7 @@ private let identifierRegex = try NSRegularExpression( private let ignoreRegex = try Regex(#"// *ignore-import$"#) private var cachedLines = [String: [String.SubSequence]]() -private func getImports(path: String, recordReader: RecordReader?) -> (Set, [String: Int]) { - guard let recordReader else { - return ([], [:]) - } - +private func getImports(path: String, recordReader: RecordReader) -> (Set, [String: Int]) { var importsToLineNumbers = [String: Int]() let lines = try! String(contentsOfFile: path).split(separator: "\n", omittingEmptySubsequences: false) cachedLines[path] = lines @@ -35,12 +31,7 @@ private func getImports(path: String, recordReader: RecordReader?) -> (Set References { - // Empty source files have units but no records - guard let recordReader else { - return References(usrs: [], typealiases: []) - } - +private func getReferences(unitReader: UnitReader, recordReader: RecordReader) -> References { var usrs = Set() var typealiasExts = Set() recordReader.forEach { (occurrence: SymbolOccurrence) in @@ -67,7 +58,7 @@ private func getReferences(unitReader: UnitReader, recordReader: RecordReader?) return References(usrs: usrs, typealiases: typealiasExts) } -private func collectUnitsAndRecords(indexStorePath: String) -> ([UnitReader], [String: RecordReader]) { +private func collectUnitsAndRecords(indexStorePath: String) -> [(UnitReader, RecordReader)] { let store: IndexStore do { store = try IndexStore(path: indexStorePath) @@ -75,36 +66,33 @@ private func collectUnitsAndRecords(indexStorePath: String) -> ([UnitReader], [S fatalError("error: failed to open index store: \(error)") } - var units: [UnitReader] = [] - var unitToRecord: [String: RecordReader] = [:] - + var unitsAndRecords: [(UnitReader, RecordReader)] = [] + var seenUnits = Set() for unitReader in store.units { if unitReader.mainFile.isEmpty { continue } - // If files are built in multiple configurations, we just pick the first one. - // Otherwise it's unclear which module(s) things are actually a part of. - if unitToRecord[unitReader.mainFile] != nil { + if seenUnits.contains(unitReader.mainFile) { continue } - units.append(unitReader) if let recordName = unitReader.recordName { do { let recordReader = try RecordReader(indexStore: store, recordName: recordName) - unitToRecord[unitReader.mainFile] = recordReader + unitsAndRecords.append((unitReader, recordReader)) + seenUnits.insert(unitReader.mainFile) } catch { fatalError("error: failed to load record: \(recordName) \(error)") } } } - if units.isEmpty { + if unitsAndRecords.isEmpty { fatalError("error: failed to load units from \(indexStorePath)") } - return (units, unitToRecord) + return unitsAndRecords } func main( @@ -118,35 +106,33 @@ func main( let pwd = FileManager.default.currentDirectoryPath var filesToDefinitions: [String: References] = [:] - let (units, unitToRecord) = collectUnitsAndRecords(indexStorePath: indexStorePath) + let unitsAndRecords = collectUnitsAndRecords(indexStorePath: indexStorePath) var modulesToUnits: [String: [UnitReader]] = [:] var allModuleNames = Set() - for unitReader in units { + for (unitReader, recordReader) in unitsAndRecords { allModuleNames.insert(unitReader.moduleName) modulesToUnits[unitReader.moduleName, default: []].append(unitReader) - if let recordReader = unitToRecord[unitReader.mainFile] { - var definedUsrs = Set() - var definedTypealiases = Set() + var definedUsrs = Set() + var definedTypealiases = Set() - recordReader.forEach { (occurrence: SymbolOccurrence) in - if occurrence.roles.contains(.definition) { - definedUsrs.insert(occurrence.symbol.usr) + recordReader.forEach { (occurrence: SymbolOccurrence) in + if occurrence.roles.contains(.definition) { + definedUsrs.insert(occurrence.symbol.usr) - if occurrence.symbol.kind == .typealias { - definedTypealiases.insert(occurrence.symbol.name) - } + if occurrence.symbol.kind == .typealias { + definedTypealiases.insert(occurrence.symbol.name) } - } - filesToDefinitions[unitReader.mainFile] = References( - usrs: definedUsrs, typealiases: definedTypealiases) } + + filesToDefinitions[unitReader.mainFile] = References( + usrs: definedUsrs, typealiases: definedTypealiases) } - for unitReader in units { + for (unitReader, recordReader) in unitsAndRecords { if let ignoredFileRegex, unitReader.mainFile.wholeMatch(of: ignoredFileRegex) != nil { continue } else if let ignoredModuleRegex, unitReader.moduleName.wholeMatch(of: ignoredModuleRegex) != nil { @@ -154,14 +140,13 @@ func main( } let (rawImports, importsToLineNumbers) = getImports( - path: unitReader.mainFile, recordReader: unitToRecord[unitReader.mainFile]) + path: unitReader.mainFile, recordReader: recordReader) let allImports = rawImports.intersection(allModuleNames) if allImports.isEmpty { continue } - let references = getReferences( - unitReader: unitReader, recordReader: unitToRecord[unitReader.mainFile]) + let references = getReferences(unitReader: unitReader, recordReader: recordReader) var usedImports = Set() for anImport in allImports { for dependentUnit in modulesToUnits[anImport] ?? [] { From b924fffb5b73e385cec7b010a0c4724d9ab4abd2 Mon Sep 17 00:00:00 2001 From: Keith Smiley Date: Fri, 31 Mar 2023 14:14:38 -0700 Subject: [PATCH 17/20] cleanup --- Sources/unused-imports/main.swift | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/Sources/unused-imports/main.swift b/Sources/unused-imports/main.swift index 8ae54d7..0a43b9f 100644 --- a/Sources/unused-imports/main.swift +++ b/Sources/unused-imports/main.swift @@ -3,7 +3,6 @@ import Darwin import Foundation private typealias References = (usrs: Set, typealiases: Set) -// FIXME: This isn't complete private let identifierRegex = try NSRegularExpression( pattern: "([a-zA-Z_][a-zA-Z0-9_]*)", options: []) private let ignoreRegex = try Regex(#"// *ignore-import$"#) @@ -43,9 +42,9 @@ private func getReferences(unitReader: UnitReader, recordReader: RecordReader) - let range = NSRange(indexes, in: line) // FIXME: extension [Int] doesn't match guard let identifierRange = identifierRegex.firstMatch(in: line, range: range)?.range(at: 1) else { - // print("no identifier line is: \(line[indexes]) in \(unitReader.mainFile)") return } + let identifier = String(line[Range(identifierRange, in: line)!]) if identifier != occurrence.symbol.name { typealiasExts.insert(identifier) From 743bd01c86a62b03900291fe1b1b8971595049d4 Mon Sep 17 00:00:00 2001 From: Keith Smiley Date: Tue, 4 Apr 2023 16:41:17 -0700 Subject: [PATCH 18/20] onlyt build --- .github/workflows/bazel.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/bazel.yml b/.github/workflows/bazel.yml index d67b3ee..3809412 100644 --- a/.github/workflows/bazel.yml +++ b/.github/workflows/bazel.yml @@ -9,5 +9,5 @@ jobs: steps: - uses: actions/checkout@v3 - uses: swift-actions/setup-swift@v1 - - run: bazelisk test //Tests/... --test_output=errors - - run: swift test + - run: bazelisk build //... + - run: swift build From 76a77fe3e6fd94f04797d8a44b2ead3b7bf58b1e Mon Sep 17 00:00:00 2001 From: Keith Smiley Date: Wed, 5 Apr 2023 11:32:46 -0700 Subject: [PATCH 19/20] Update Sources/unused-imports/main.swift Co-authored-by: Scott Berrevoets --- Sources/unused-imports/main.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Sources/unused-imports/main.swift b/Sources/unused-imports/main.swift index 0a43b9f..157f626 100644 --- a/Sources/unused-imports/main.swift +++ b/Sources/unused-imports/main.swift @@ -40,7 +40,7 @@ private func getReferences(unitReader: UnitReader, recordReader: RecordReader) - let line = String(lines[occurrence.location.line - 1]) let indexes = line.index(line.startIndex, offsetBy: occurrence.location.column - 1).. Date: Wed, 5 Apr 2023 11:41:46 -0700 Subject: [PATCH 20/20] Regex --- Sources/unused-imports/main.swift | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/Sources/unused-imports/main.swift b/Sources/unused-imports/main.swift index 157f626..e8cfcd8 100644 --- a/Sources/unused-imports/main.swift +++ b/Sources/unused-imports/main.swift @@ -3,8 +3,7 @@ import Darwin import Foundation private typealias References = (usrs: Set, typealiases: Set) -private let identifierRegex = try NSRegularExpression( - pattern: "([a-zA-Z_][a-zA-Z0-9_]*)", options: []) +private let identifierRegex = try Regex("([a-zA-Z_][a-zA-Z0-9_]*)") private let ignoreRegex = try Regex(#"// *ignore-import$"#) private var cachedLines = [String: [String.SubSequence]]() @@ -37,15 +36,14 @@ private func getReferences(unitReader: UnitReader, recordReader: RecordReader) - if occurrence.symbol.subkind == .swiftExtensionOfStruct { usrs.insert(occurrence.symbol.usr) let lines = cachedLines[unitReader.mainFile]! - let line = String(lines[occurrence.location.line - 1]) - let indexes = line.index(line.startIndex, offsetBy: occurrence.location.column - 1)..