Add warning for superfluous ignore comments (#427)

.mise/tasks/scripts/helpers.sh

@@ -0,0 +1,8 @@
+# Shared helper functions for mise tasks
+
+# Formats a JSON baseline file in place
+format_baseline() {
+    local file="$1"
+    jq . "$file" > "$file.tmp"
+    mv "$file.tmp" "$file"
+}

.mise/tasks/write-baseline-bazel

@@ -0,0 +1,8 @@
+#!/bin/bash
+
+set -euo pipefail
+
+source "$(dirname "$0")/scripts/helpers.sh"
+
+bazel run :periphery -- scan --bazel --write-baseline baselines/bazel.json
+format_baseline baselines/bazel.json

.mise/tasks/write-baseline-linux

@@ -1,11 +1,12 @@
 #!/bin/bash
 
-set -e
+set -euo pipefail
+
+source "$(dirname "$0")/scripts/helpers.sh"
 
 export DOCKER_CLI_HINTS=false
 docker build -t periphery -f docker/Dockerfile-Linux .
 docker run --name periphery_write_baseline -t periphery scan "$@" --write-baseline /baseline.json
 docker cp periphery_write_baseline:baseline.json ./baselines/linux.json
 docker rm periphery_write_baseline
-jq . ./baselines/linux.json > ./baselines/linux.json.tmp
-mv ./baselines/linux.json.tmp ./baselines/linux.json
+format_baseline baselines/linux.json

.mise/tasks/write-baseline-linux-bazel

@@ -1,11 +1,12 @@
 #!/bin/bash
 
-set -e
+set -euo pipefail
+
+source "$(dirname "$0")/scripts/helpers.sh"
 
 export DOCKER_CLI_HINTS=false
 docker build -t periphery -f docker/Dockerfile-Bazel .
 docker run --name periphery_write_baseline -t periphery scan "$@" --bazel --write-baseline /baseline.json
 docker cp periphery_write_baseline:baseline.json ./baselines/linux-bazel.json
 docker rm periphery_write_baseline
-jq . ./baselines/linux-bazel.json > ./baselines/linux-bazel.json.tmp
-mv ./baselines/linux-bazel.json.tmp ./baselines/linux-bazel.json
+format_baseline baselines/linux-bazel.json

CHANGELOG.md

@@ -7,6 +7,7 @@
 ##### Enhancements
 
 - The `--color` option now accepts one of `auto`, `always` and `never`. In `auto` mode, color is disabled for dumb terminals and non-TTYs.
+- Added detection of superfluous `// periphery:ignore` comments. A warning is now reported when an ignore comment is unnecessary because the declaration is actually used.
 
 ##### Bug Fixes
 

Sources/Extensions/String+Version.swift

@@ -1,5 +1,3 @@
-// periphery:ignore:all
-
 import Foundation
 
 public typealias VersionString = String

Sources/Frontend/Commands/ScanCommand.swift

@@ -262,6 +262,7 @@ struct ScanCommand: ParsableCommand {
 
         if let baselinePath = configuration.writeBaseline {
             let usrs = filteredResults
+                .filter(\.includeInBaseline)
                 .flatMapSet { $0.usrs }
                 .union(baseline?.usrs ?? [])
             let baseline = Baseline.v1(usrs: usrs.sorted())

Sources/Indexer/SwiftIndexer.swift

@@ -371,10 +371,10 @@ final class SwiftIndexer: Indexer {
             let fileCommands = syntaxVisitor.parseComments()
 
             if fileCommands.contains(.ignoreAll) {
-                retainHierarchy(declarations)
+                retainHierarchy(declarations, markExplicitlyIgnored: true)
             } else {
                 for decl in declarations where decl.commentCommands.contains(.ignore) {
-                    retainHierarchy([decl])
+                    retainHierarchy([decl], markExplicitlyIgnored: true)
                 }
             }
         }
@@ -399,6 +399,7 @@ final class SwiftIndexer: Indexer {
                 decl.modifiers = Set(result.modifiers)
                 decl.commentCommands = Set(result.commentCommands)
                 decl.declaredType = result.variableType
+
                 decl.hasGenericFunctionReturnedMetatypeParameters = result.hasGenericFunctionReturnedMetatypeParameters
 
                 for ref in decl.references.union(decl.related) {
@@ -433,13 +434,17 @@ final class SwiftIndexer: Indexer {
             }
         }
 
-        private func retainHierarchy(_ decls: [Declaration]) {
+        private func retainHierarchy(_ decls: [Declaration], markExplicitlyIgnored: Bool = false) {
             for decl in decls {
                 graph.withLock { graph in
                     graph.markRetained(decl)
                     decl.unusedParameters.forEach { graph.markRetained($0) }
+                    if markExplicitlyIgnored {
+                        graph.markExplicitlyIgnored(decl)
+                        decl.unusedParameters.forEach { graph.markExplicitlyIgnored($0) }
+                    }
                 }
-                retainHierarchy(Array(decl.declarations))
+                retainHierarchy(Array(decl.declarations), markExplicitlyIgnored: markExplicitlyIgnored)
             }
         }
 
@@ -465,6 +470,17 @@ final class SwiftIndexer: Indexer {
                 $0[$1.location] = $1
             }
 
+            // Build a map of ignored param names per function, and track functions with ignored
+            // params so ScanResultBuilder can efficiently detect superfluous ignores.
+            var ignoredParamsByLocation: [Location: [String]] = [:]
+            for functionDecl in functionDecls {
+                let ignoredParamNames = functionDecl.commentCommands.ignoredParameterNames
+                if !ignoredParamNames.isEmpty {
+                    ignoredParamsByLocation[functionDecl.location] = ignoredParamNames
+                    graph.withLock { $0.markHasIgnoredParameters(functionDecl) }
+                }
+            }
+
             let analyzer = UnusedParameterAnalyzer()
             let paramsByFunction = analyzer.analyze(
                 file: syntaxVisitor.sourceFile,
@@ -480,14 +496,7 @@ final class SwiftIndexer: Indexer {
                     continue
                 }
 
-                let ignoredParamNames = functionDecl.commentCommands.flatMap { command -> [String] in
-                    switch command {
-                    case let .ignoreParameters(params):
-                        return params
-                    default:
-                        return []
-                    }
-                }
+                let ignoredParamNames = ignoredParamsByLocation[functionDecl.location] ?? []
 
                 graph.withLock { graph in
                     for param in params {
@@ -501,6 +510,7 @@ final class SwiftIndexer: Indexer {
 
                         if (functionDecl.isObjcAccessible && configuration.retainObjcAccessible) || ignoredParamNames.contains(param.name.text) {
                             graph.markRetained(paramDecl)
+                            graph.markExplicitlyIgnored(paramDecl)
                         }
                     }
                 }

Sources/PeripheryKit/Results/OutputFormatter.swift

@@ -33,6 +33,8 @@ extension OutputFormatter {
             "redundantProtocol"
         case .redundantPublicAccessibility:
             "redundantPublicAccessibility"
+        case .superfluousIgnoreCommand:
+            "superfluousIgnoreCommand"
         }
     }
 
@@ -64,6 +66,8 @@ 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 .superfluousIgnoreCommand:
+                description += "Superfluous ignore comment for \(kindDisplayName) '\(name)' (declaration is referenced and should not be ignored)"
             }
         } else {
             description += "Unused"

Sources/PeripheryKit/ScanResult.swift

@@ -7,6 +7,7 @@ public struct ScanResult {
         case assignOnlyProperty
         case redundantProtocol(references: Set<Reference>, inherited: Set<String>)
         case redundantPublicAccessibility(modules: Set<String>)
+        case superfluousIgnoreCommand
     }
 
     let declaration: Declaration
@@ -15,4 +16,12 @@ public struct ScanResult {
     public var usrs: Set<String> {
         declaration.usrs
     }
+
+    /// Indicates whether this result should be included in baselines.
+    /// Superfluous ignore command results are excluded since they're warnings
+    /// about unnecessary comments, not unused code.
+    public var includeInBaseline: Bool {
+        if case .superfluousIgnoreCommand = annotation { return false }
+        return true
+    }
 }

Sources/PeripheryKit/ScanResultBuilder.swift

@@ -40,17 +40,90 @@ public enum ScanResultBuilder {
         let annotatedRedundantPublicAccessibility: [ScanResult] = redundantPublicAccessibility.map {
             .init(declaration: $0.0, annotation: .redundantPublicAccessibility(modules: $0.1))
         }
+
+        // Detect superfluous ignore commands
+        // 1. Declarations with ignore comments that have references from non-ignored code
+        let superfluousDeclarations = graph.explicitlyIgnoredDeclarations
+            .filter { decl in
+                hasReferencesFromNonIgnoredCode(decl, graph: graph)
+            }
+
+        // 2. Parameters with ignore comments that are actually used (not in unusedParameters)
+        let superfluousParamResults = findSuperfluousParameterIgnores(graph: graph)
+
+        let annotatedSuperfluousIgnoreCommands: [ScanResult] = superfluousDeclarations
+            .map { .init(declaration: $0, annotation: .superfluousIgnoreCommand) }
+            + superfluousParamResults
+
         let allAnnotatedDeclarations = annotatedRemovableDeclarations +
             annotatedAssignOnlyProperties +
             annotatedRedundantProtocols +
-            annotatedRedundantPublicAccessibility
+            annotatedRedundantPublicAccessibility +
+            annotatedSuperfluousIgnoreCommands
 
         return allAnnotatedDeclarations
-            .filter {
-                !$0.declaration.isImplicit &&
-                    !$0.declaration.kind.isAccessorKind &&
-                    !graph.ignoredDeclarations.contains($0.declaration) &&
-                    !graph.retainedDeclarations.contains($0.declaration)
+            .filter { result in
+                guard !result.declaration.isImplicit,
+                      !result.declaration.kind.isAccessorKind,
+                      !graph.ignoredDeclarations.contains(result.declaration)
+                else { return false }
+
+                // Superfluous ignore command results must bypass the retained filter below.
+                // Declarations with ignore comments are always in retainedDeclarations (that's
+                // how ignore comments work), so filtering them out would prevent us from ever
+                // reporting superfluous ignore commands.
+                if case .superfluousIgnoreCommand = result.annotation {
+                    return true
+                }
+
+                return !graph.retainedDeclarations.contains(result.declaration)
+            }
+    }
+
+    /// Checks if a declaration has references from code that is not part of the explicitly ignored set.
+    /// This indicates that the declaration would have been marked as used even without the ignore command.
+    private static func hasReferencesFromNonIgnoredCode(_ decl: Declaration, graph: SourceGraph) -> Bool {
+        let references = graph.references(to: decl)
+
+        for ref in references {
+            guard let parent = ref.parent else { continue }
+
+            // Check if the parent is not in the explicitly ignored set.
+            // This covers deeply nested declarations because retainHierarchy marks
+            // the entire descendant tree as explicitly ignored, not just the root.
+            if !graph.explicitlyIgnoredDeclarations.contains(parent) {
+                // Also check that the parent is actually used (not itself unused)
+                if graph.usedDeclarations.contains(parent) {
+                    return true
+                }
+            }
+        }
+
+        return false
+    }
+
+    /// Finds parameters that have `// periphery:ignore:parameters` comments but are actually used.
+    /// If a parameter is ignored but NOT in unusedParameters, it means it's used and the ignore is superfluous.
+    private static func findSuperfluousParameterIgnores(graph: SourceGraph) -> [ScanResult] {
+        var results: [ScanResult] = []
+
+        for decl in graph.functionsWithIgnoredParameters {
+            let ignoredParamNames = decl.commentCommands.ignoredParameterNames
+            let unusedParamNames = Set(decl.unusedParameters.compactMap(\.name))
+
+            for ignoredParamName in ignoredParamNames {
+                if !unusedParamNames.contains(ignoredParamName) {
+                    // The ignored parameter is actually used - create a result for it
+                    let parentUsrs = decl.usrs.sorted().joined(separator: "-")
+                    let usr = "param-\(ignoredParamName)-\(decl.name ?? "unknown-function")-\(parentUsrs)"
+                    let paramDecl = Declaration(kind: .varParameter, usrs: [usr], location: decl.location)
+                    paramDecl.name = ignoredParamName
+                    paramDecl.parent = decl
+                    results.append(.init(declaration: paramDecl, annotation: .superfluousIgnoreCommand))
+                }
             }
+        }
+
+        return results
     }
 }

Sources/SourceGraph/Elements/CommentCommand.swift

@@ -77,4 +77,15 @@ public extension Sequence<CommentCommand> {
 
         return nil
     }
+
+    var ignoredParameterNames: [String] {
+        flatMap { command -> [String] in
+            switch command {
+            case let .ignoreParameters(params):
+                params
+            default:
+                []
+            }
+        }
+    }
 }

Sources/SourceGraph/SourceGraph.swift

@@ -20,6 +20,8 @@ public final class SourceGraph {
     public private(set) var unusedModuleImports: Set<Declaration> = []
     public private(set) var assignOnlyProperties: Set<Declaration> = []
     public private(set) var extensions: [Declaration: Set<Declaration>] = [:]
+    public private(set) var explicitlyIgnoredDeclarations: Set<Declaration> = []
+    public private(set) var functionsWithIgnoredParameters: Set<Declaration> = []
 
     private var indexedModules: Set<String> = []
     private var unindexedExportedModules: Set<String> = []
@@ -89,6 +91,14 @@ public final class SourceGraph {
         _ = ignoredDeclarations.insert(declaration)
     }
 
+    public func markExplicitlyIgnored(_ declaration: Declaration) {
+        _ = explicitlyIgnoredDeclarations.insert(declaration)
+    }
+
+    public func markHasIgnoredParameters(_ declaration: Declaration) {
+        _ = functionsWithIgnoredParameters.insert(declaration)
+    }
+
     public func markRetained(_ declaration: Declaration) {
         _ = retainedDeclarations.insert(declaration)
     }

Sources/SourceGraph/SourceGraphDebugger.swift

@@ -1,5 +1,3 @@
-// periphery:ignore:all
-
 import Foundation
 
 final class SourceGraphDebugger {

Sources/SyntaxAnalysis/CommentCommand.swift

@@ -4,27 +4,29 @@ import SystemPackage
 
 extension CommentCommand {
     static func parseCommands(in trivia: Trivia?) -> [CommentCommand] {
-        let comments: [String] = trivia?.compactMap {
-            switch $0 {
-            case let .lineComment(comment),
-                 let .blockComment(comment),
-                 let .docLineComment(comment),
-                 let .docBlockComment(comment):
-                comment
-            default:
-                nil
+        trivia?.compactMap { piece -> CommentCommand? in
+            // Extract comment text and marker length based on trivia piece kind
+            let parsed: (comment: String, markerLength: Int)? = switch piece {
+            case let .lineComment(text): (text, 2) // //
+            case let .docLineComment(text): (text, 3) // ///
+            case let .blockComment(text): (text, 2) // /*
+            case let .docBlockComment(text): (text, 3) // /**
+            default: nil
             }
-        } ?? []
 
-        return comments
-            .compactMap { comment in
-                guard let range = comment.range(of: "periphery:") else { return nil }
+            guard let (comment, markerLength) = parsed,
+                  let range = comment.range(of: "periphery:") else { return nil }
 
-                var rawCommand = String(comment[range.upperBound...]).replacingOccurrences(of: "*/", with: "").trimmed
-                // Anything after '-' in a comment command is ignored.
-                rawCommand = String(rawCommand.split(separator: "-").first ?? "").trimmed
-                return CommentCommand.parse(rawCommand)
-            }
+            // Only respect commands at the start of a comment (after the marker and whitespace).
+            let prefixStart = comment.index(comment.startIndex, offsetBy: markerLength)
+            let prefixBeforeCommand = String(comment[prefixStart ..< range.lowerBound])
+            guard prefixBeforeCommand.trimmingCharacters(in: .whitespaces).isEmpty else { return nil }
+
+            var rawCommand = String(comment[range.upperBound...]).replacingOccurrences(of: "*/", with: "").trimmed
+            // Anything after '-' in a comment command is ignored.
+            rawCommand = String(rawCommand.split(separator: "-").first ?? "").trimmed
+            return CommentCommand.parse(rawCommand)
+        } ?? []
     }
 
     static func parse(_ rawCommand: String) -> Self? {