diff --git a/.mise/tasks/scripts/helpers.sh b/.mise/tasks/scripts/helpers.sh new file mode 100644 index 000000000..86e6df8fd --- /dev/null +++ b/.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" +} diff --git a/.mise/tasks/write-baseline-bazel b/.mise/tasks/write-baseline-bazel new file mode 100755 index 000000000..cc1e7384e --- /dev/null +++ b/.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 diff --git a/.mise/tasks/write-baseline-linux b/.mise/tasks/write-baseline-linux index 6e373a715..0d9a1e20b 100755 --- a/.mise/tasks/write-baseline-linux +++ b/.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 diff --git a/.mise/tasks/write-baseline-linux-bazel b/.mise/tasks/write-baseline-linux-bazel index bffbd617c..05413c9f4 100755 --- a/.mise/tasks/write-baseline-linux-bazel +++ b/.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 diff --git a/CHANGELOG.md b/CHANGELOG.md index c0c7a0abb..1b0c59954 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/MODULE.bazel.lock b/MODULE.bazel.lock index 11fa27a61..8621f3a50 100644 --- a/MODULE.bazel.lock +++ b/MODULE.bazel.lock @@ -193,7 +193,7 @@ "//bazel:generated.bzl%generated": { "general": { "bzlTransitiveDigest": "nMR2FBcoRPImVocN9DNOnm2NQWyTbJPu7SHJgAXsLFw=", - "usagesDigest": "Hdmxc+LdckmZqHgeSNbQlCU0uQaBARRWkeCpjro+Xhk=", + "usagesDigest": "S5zTCGsw3nugb0U3s6bPvFYl04dzqTMrLLUzzabunO8=", "recordedFileInputs": {}, "recordedDirentsInputs": {}, "envVariables": {}, diff --git a/Sources/Extensions/String+Version.swift b/Sources/Extensions/String+Version.swift index 42006b3f3..07ad33916 100644 --- a/Sources/Extensions/String+Version.swift +++ b/Sources/Extensions/String+Version.swift @@ -1,5 +1,3 @@ -// periphery:ignore:all - import Foundation public typealias VersionString = String diff --git a/Sources/Frontend/Commands/ScanCommand.swift b/Sources/Frontend/Commands/ScanCommand.swift index 14d1a743c..373250166 100644 --- a/Sources/Frontend/Commands/ScanCommand.swift +++ b/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()) diff --git a/Sources/Indexer/SwiftIndexer.swift b/Sources/Indexer/SwiftIndexer.swift index 9f4f28447..2a8dfbadf 100644 --- a/Sources/Indexer/SwiftIndexer.swift +++ b/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) } } } diff --git a/Sources/PeripheryKit/Results/OutputFormatter.swift b/Sources/PeripheryKit/Results/OutputFormatter.swift index 42cfac01e..33b861632 100644 --- a/Sources/PeripheryKit/Results/OutputFormatter.swift +++ b/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" diff --git a/Sources/PeripheryKit/ScanResult.swift b/Sources/PeripheryKit/ScanResult.swift index cc346d3c6..e558270db 100644 --- a/Sources/PeripheryKit/ScanResult.swift +++ b/Sources/PeripheryKit/ScanResult.swift @@ -7,6 +7,7 @@ public struct ScanResult { case assignOnlyProperty case redundantProtocol(references: Set, inherited: Set) case redundantPublicAccessibility(modules: Set) + case superfluousIgnoreCommand } let declaration: Declaration @@ -15,4 +16,12 @@ public struct ScanResult { public var usrs: Set { 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 + } } diff --git a/Sources/PeripheryKit/ScanResultBuilder.swift b/Sources/PeripheryKit/ScanResultBuilder.swift index fc3d1a053..97d632e71 100644 --- a/Sources/PeripheryKit/ScanResultBuilder.swift +++ b/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 } } diff --git a/Sources/SourceGraph/Elements/CommentCommand.swift b/Sources/SourceGraph/Elements/CommentCommand.swift index 068659436..361cdf112 100644 --- a/Sources/SourceGraph/Elements/CommentCommand.swift +++ b/Sources/SourceGraph/Elements/CommentCommand.swift @@ -77,4 +77,15 @@ public extension Sequence { return nil } + + var ignoredParameterNames: [String] { + flatMap { command -> [String] in + switch command { + case let .ignoreParameters(params): + params + default: + [] + } + } + } } diff --git a/Sources/SourceGraph/SourceGraph.swift b/Sources/SourceGraph/SourceGraph.swift index 6c06e0e12..0d5538e24 100644 --- a/Sources/SourceGraph/SourceGraph.swift +++ b/Sources/SourceGraph/SourceGraph.swift @@ -20,6 +20,8 @@ public final class SourceGraph { public private(set) var unusedModuleImports: Set = [] public private(set) var assignOnlyProperties: Set = [] public private(set) var extensions: [Declaration: Set] = [:] + public private(set) var explicitlyIgnoredDeclarations: Set = [] + public private(set) var functionsWithIgnoredParameters: Set = [] private var indexedModules: Set = [] private var unindexedExportedModules: Set = [] @@ -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) } diff --git a/Sources/SourceGraph/SourceGraphDebugger.swift b/Sources/SourceGraph/SourceGraphDebugger.swift index 538417f57..e62f6ddba 100644 --- a/Sources/SourceGraph/SourceGraphDebugger.swift +++ b/Sources/SourceGraph/SourceGraphDebugger.swift @@ -1,5 +1,3 @@ -// periphery:ignore:all - import Foundation final class SourceGraphDebugger { diff --git a/Sources/SyntaxAnalysis/CommentCommand.swift b/Sources/SyntaxAnalysis/CommentCommand.swift index 1c0dec7c1..7379a763d 100644 --- a/Sources/SyntaxAnalysis/CommentCommand.swift +++ b/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? { diff --git a/Sources/SyntaxAnalysis/DeclarationSyntaxVisitor.swift b/Sources/SyntaxAnalysis/DeclarationSyntaxVisitor.swift index 704cb748f..985dad60a 100644 --- a/Sources/SyntaxAnalysis/DeclarationSyntaxVisitor.swift +++ b/Sources/SyntaxAnalysis/DeclarationSyntaxVisitor.swift @@ -578,18 +578,18 @@ private extension SyntaxProtocol { /// Matches uses like these: /// /// ``` - /// // periphery:ignore + /// // ignore /// Foo { /// } /// ``` /// /// ``` - /// Foo { // periphery:ignore + /// Foo { // ignore /// } /// ``` /// /// ``` - /// Foo {} // periphery:ignore + /// Foo {} // ignore /// ``` var commentCommandTrivia: Trivia { var commandTrivia = leadingTrivia.merging(trailingTrivia) diff --git a/Tests/Fixtures/Sources/RetentionFixtures/testSuperfluousIgnoreCommand.swift b/Tests/Fixtures/Sources/RetentionFixtures/testSuperfluousIgnoreCommand.swift new file mode 100644 index 000000000..f82cbea0f --- /dev/null +++ b/Tests/Fixtures/Sources/RetentionFixtures/testSuperfluousIgnoreCommand.swift @@ -0,0 +1,94 @@ +import Foundation + +// MARK: - Superfluous ignore command (function is actually used) + +// periphery:ignore +public func superfluouslyIgnoredFunc() { + print("I am actually used!") +} + +// This function calls the ignored one, making the ignore superfluous +public func callerOfSuperfluouslyIgnoredFunc() { + superfluouslyIgnoredFunc() +} + +// MARK: - Superfluous ignore command on class (class is used) + +// periphery:ignore +public class SuperfluouslyIgnoredClass { + public func someMethod() {} +} + +public func useSuperfluouslyIgnoredClass() { + _ = SuperfluouslyIgnoredClass() +} + +// MARK: - Non-superfluous ignore command (function is NOT used) + +// periphery:ignore +public func correctlyIgnoredFunc() { + print("I am NOT used anywhere!") +} + +// MARK: - Non-superfluous ignore command (class is NOT used) + +// periphery:ignore +public class CorrectlyIgnoredClass { + public func someMethod() {} +} + +// MARK: - Ignored declaration within non-ignored parent + +public class NonIgnoredParentClass { + // This method is ignored but actually used - superfluous + // periphery:ignore + public func superfluouslyIgnoredMethod() { + print("I am used!") + } + + // This method is ignored and NOT used - correctly ignored + // periphery:ignore + public func correctlyIgnoredMethod() { + print("I am not used!") + } + + public func callerMethod() { + superfluouslyIgnoredMethod() + } +} + +// MARK: - Deeply nested declarations within ignored hierarchy + +// When an entire class is ignored, internal references between its members +// should NOT make those members appear superfluously ignored. +// periphery:ignore +public class DeeplyNestedIgnoredClass { + public func methodA() { + methodB() // Internal reference - should NOT make methodB superfluous + } + + public func methodB() { + methodC() // Internal reference - should NOT make methodC superfluous + } + + public func methodC() { + print("Deeply nested") + } +} + +// MARK: - Superfluous ignore for parameters + +public class ParameterIgnoreClass { + // This parameter is ignored but actually used - superfluous + // periphery:ignore:parameters usedParam + public func superfluousParamIgnore(usedParam: String) { + print(usedParam) // Parameter IS used, so ignore is superfluous + } + + // This parameter is ignored and NOT used - correctly ignored + // periphery:ignore:parameters unusedParam + public func correctParamIgnore(unusedParam: String) { + print("Not using the param") + } +} + diff --git a/Tests/PeripheryTests/RetentionTest.swift b/Tests/PeripheryTests/RetentionTest.swift index 98561b46f..294963cc8 100644 --- a/Tests/PeripheryTests/RetentionTest.swift +++ b/Tests/PeripheryTests/RetentionTest.swift @@ -1173,6 +1173,52 @@ final class RetentionTest: FixtureSourceGraphTestCase { } } + func testSuperfluousIgnoreCommand() { + analyze(retainPublic: true) { + // These have ignore commands but are actually used, so the ignore is superfluous + assertSuperfluousIgnoreCommand(.functionFree("superfluouslyIgnoredFunc()")) + assertSuperfluousIgnoreCommand(.class("SuperfluouslyIgnoredClass")) + + // These have ignore commands and are NOT used, so the ignore is needed + assertNotSuperfluousIgnoreCommand(.functionFree("correctlyIgnoredFunc()")) + assertNotSuperfluousIgnoreCommand(.class("CorrectlyIgnoredClass")) + + // The callers should be referenced normally + assertReferenced(.functionFree("callerOfSuperfluouslyIgnoredFunc()")) + assertReferenced(.functionFree("useSuperfluouslyIgnoredClass()")) + + // Test ignored declarations within non-ignored parent + assertReferenced(.class("NonIgnoredParentClass")) { + // This method is ignored but used by callerMethod - superfluous + self.assertSuperfluousIgnoreCommand(.functionMethodInstance("superfluouslyIgnoredMethod()")) + // This method is ignored and not used - correctly ignored + self.assertNotSuperfluousIgnoreCommand(.functionMethodInstance("correctlyIgnoredMethod()")) + // The caller method should be referenced normally + self.assertReferenced(.functionMethodInstance("callerMethod()")) + } + + // Test deeply nested declarations within ignored hierarchy. + // Internal references between members of an ignored class should NOT + // make those members appear superfluously ignored. + assertNotSuperfluousIgnoreCommand(.class("DeeplyNestedIgnoredClass")) + assertNotSuperfluousIgnoreCommand(.functionMethodInstance("methodA()")) + assertNotSuperfluousIgnoreCommand(.functionMethodInstance("methodB()")) + assertNotSuperfluousIgnoreCommand(.functionMethodInstance("methodC()")) + + // Test superfluous ignore for parameters + assertReferenced(.class("ParameterIgnoreClass")) { + self.assertReferenced(.functionMethodInstance("superfluousParamIgnore(usedParam:)")) { + // Parameter is ignored but actually used - superfluous + self.assertSuperfluousIgnoreCommand(.varParameter("usedParam")) + } + self.assertReferenced(.functionMethodInstance("correctParamIgnore(unusedParam:)")) { + // Parameter is ignored and not used - correctly ignored + self.assertNotSuperfluousIgnoreCommand(.varParameter("unusedParam")) + } + } + } + } + // MARK: - Swift Testing #if canImport(Testing) diff --git a/Tests/PeripheryTests/Syntax/CommentCommandTest.swift b/Tests/PeripheryTests/Syntax/CommentCommandTest.swift new file mode 100644 index 000000000..f3136b948 --- /dev/null +++ b/Tests/PeripheryTests/Syntax/CommentCommandTest.swift @@ -0,0 +1,76 @@ +import Foundation +@testable import SourceGraph +import SwiftSyntax +@testable import SyntaxAnalysis +import XCTest + +final class CommentCommandTest: XCTestCase { + func testParseIgnore() { + assertParsesCommand("// periphery:ignore", expected: .ignore) + assertParsesCommand("/// periphery:ignore", expected: .ignore) + assertParsesCommand("/* periphery:ignore */", expected: .ignore) + assertParsesCommand("/** periphery:ignore */", expected: .ignore) + } + + func testParseIgnoreAll() { + assertParsesCommand("// periphery:ignore:all", expected: .ignoreAll) + } + + func testParseIgnoreParameters() { + assertParsesCommand("// periphery:ignore:parameters foo", expected: .ignoreParameters(["foo"])) + assertParsesCommand("// periphery:ignore:parameters foo,bar", expected: .ignoreParameters(["foo", "bar"])) + } + + func testAllowsLeadingWhitespace() { + assertParsesCommand("// periphery:ignore", expected: .ignore) + assertParsesCommand("/// periphery:ignore", expected: .ignore) + assertParsesCommand("/* periphery:ignore */", expected: .ignore) + } + + func testIgnoresCommandsWithPrecedingText() { + assertDoesNotParseCommand("// some text periphery:ignore") + assertDoesNotParseCommand("/// Docs about periphery:ignore") + assertDoesNotParseCommand("// `periphery:ignore` is used to ignore") + assertDoesNotParseCommand("/* text periphery:ignore */") + } + + func testTrailingCommentAfterHyphen() { + // Anything after '-' is treated as a trailing comment and ignored + assertParsesCommand("// periphery:ignore - this is a reason for ignoring", expected: .ignore) + assertParsesCommand("// periphery:ignore:all - ignore entire file", expected: .ignoreAll) + assertParsesCommand("// periphery:ignore:parameters foo - param is unused intentionally", expected: .ignoreParameters(["foo"])) + } + + // MARK: - Helpers + + private func assertParsesCommand(_ comment: String, expected: CommentCommand, file: StaticString = #file, line: UInt = #line) { + let trivia = parseTrivia(comment) + let commands = CommentCommand.parseCommands(in: trivia) + XCTAssertEqual(commands.count, 1, "Expected exactly one command from '\(comment)'", file: file, line: line) + if let command = commands.first { + XCTAssertEqual(command, expected, "Command mismatch for '\(comment)'", file: file, line: line) + } + } + + private func assertDoesNotParseCommand(_ comment: String, file: StaticString = #file, line: UInt = #line) { + let trivia = parseTrivia(comment) + let commands = CommentCommand.parseCommands(in: trivia) + XCTAssertTrue(commands.isEmpty, "Expected no commands from '\(comment)', but got: \(commands)", file: file, line: line) + } + + private func parseTrivia(_ comment: String) -> Trivia { + // Determine the trivia piece type based on comment prefix + let piece: TriviaPiece = if comment.hasPrefix("///") { + .docLineComment(comment) + } else if comment.hasPrefix("//") { + .lineComment(comment) + } else if comment.hasPrefix("/**") { + .docBlockComment(comment) + } else if comment.hasPrefix("/*") { + .blockComment(comment) + } else { + .lineComment(comment) + } + return Trivia(pieces: [piece]) + } +} diff --git a/Tests/Shared/SourceGraphTestCase.swift b/Tests/Shared/SourceGraphTestCase.swift index f3abf396e..09dc21b92 100644 --- a/Tests/Shared/SourceGraphTestCase.swift +++ b/Tests/Shared/SourceGraphTestCase.swift @@ -209,6 +209,44 @@ open class SourceGraphTestCase: XCTestCase { scopeStack.removeLast() } + func assertSuperfluousIgnoreCommand(_ description: DeclarationDescription, file: StaticString = #file, line: UInt = #line) { + // For parameters, we need to check results directly since they're created at result-building time + if description.kind == .varParameter { + let found = Self.results.superfluousIgnoreCommandDeclarations.contains { + $0.kind == description.kind && $0.name == description.name + } + if !found { + XCTFail("Expected superfluous ignore command for parameter: \(description.name)", file: file, line: line) + } + return + } + + guard let declaration = materialize(description, in: Self.allIndexedDeclarations, file: file, line: line) else { return } + + if !Self.results.superfluousIgnoreCommandDeclarations.contains(declaration) { + XCTFail("Expected declaration to have superfluous ignore command: \(declaration)", file: file, line: line) + } + } + + func assertNotSuperfluousIgnoreCommand(_ description: DeclarationDescription, file: StaticString = #file, line: UInt = #line) { + // For parameters, we need to check results directly since they're created at result-building time + if description.kind == .varParameter { + let found = Self.results.superfluousIgnoreCommandDeclarations.contains { + $0.kind == description.kind && $0.name == description.name + } + if found { + XCTFail("Expected no superfluous ignore command for parameter: \(description.name)", file: file, line: line) + } + return + } + + guard let declaration = materialize(description, in: Self.allIndexedDeclarations, file: file, line: line) else { return } + + if Self.results.superfluousIgnoreCommandDeclarations.contains(declaration) { + XCTFail("Expected declaration to not have superfluous ignore command: \(declaration)", file: file, line: line) + } + } + func assertOverrides(_ description: DeclarationDescription, _ overrides: [CommentCommand.Override], file: StaticString = #file, line: UInt = #line) { guard let declaration = materialize(description, file: file, line: line) else { XCTFail("Failed to materialize \(description)", file: file, line: line) @@ -312,4 +350,14 @@ private extension [ScanResult] { return nil } } + + var superfluousIgnoreCommandDeclarations: Set { + compactMapSet { + if case .superfluousIgnoreCommand = $0.annotation { + return $0.declaration + } + + return nil + } + } } diff --git a/baselines/bazel.json b/baselines/bazel.json index 33393450d..835493c5d 100644 --- a/baselines/bazel.json +++ b/baselines/bazel.json @@ -1,13 +1,10 @@ { - "v1": { - "usrs": [ - "s:13Configuration15AbstractSettingP5resetyyF", - "s:13Configuration7SettingC5resetyyF", - "s:13ConfigurationAAC13resetMatchersyyF", - "s:13ConfigurationAAC5resetyyF", - "s:13SystemPackage8FilePathV10ExtensionsE5chdir7closureyyyKXE_tKF", - "s:14SyntaxAnalysis21UnusedParameterParserV5parse4file0F9ProtocolsSayAA8FunctionVG11SourceGraph0J4FileC_SbtKFZ", - "s:14SyntaxAnalysis8FunctionV8fullNameSSvp" - ] - } -} \ No newline at end of file + "v1": { + "usrs": [ + "s:11SourceGraph0aB8DebuggerC", + "s:13SystemPackage8FilePathV10ExtensionsE5chdir7closureyyyKXE_tKF", + "s:14SyntaxAnalysis21UnusedParameterParserV5parse4file0F9ProtocolsSayAA8FunctionVG11SourceGraph0J4FileC_SbtKFZ", + "s:14SyntaxAnalysis8FunctionV8fullNameSSvp" + ] + } +} diff --git a/baselines/linux-bazel.json b/baselines/linux-bazel.json index c54f8ef14..f55df1d7e 100644 --- a/baselines/linux-bazel.json +++ b/baselines/linux-bazel.json @@ -2,6 +2,7 @@ "v1": { "usrs": [ "s:10Extensions4Glob33_78772790CB7745B67917FD65D1BCE611LLC", + "s:11SourceGraph0aB8DebuggerC", "s:11SourceGraph15ProjectFileKindO10extensionsSaySSGvp", "s:13SystemPackage8FilePathV10ExtensionsE4globyShyACGSSFZ", "s:13SystemPackage8FilePathV10ExtensionsE5chdir7closureyyyKXE_tKF",