From 45bf2b87da81ef9c39eeb8edb5703da8730850c4 Mon Sep 17 00:00:00 2001 From: Doug Gregor Date: Fri, 30 Aug 2024 14:46:40 -0700 Subject: [PATCH 1/7] Introduce the ability to retain compiler and feature checks when removing inactive regions The compiler's handling of inlinable text for Swift interface checking requires the ability to retain `#if`s involving compiler checks (e.g., `#if compiler(>=6.0)`) and `$`-based feature checks (`#if $AsyncAwait`) when stripping inactive regions. Expand the `removingInactive` function with a parameter that implements this behavior. --- .../SwiftIfConfig/ActiveSyntaxRewriter.swift | 86 +++++++++++++++++-- Tests/SwiftIfConfigTest/VisitorTests.swift | 52 ++++++++++- 2 files changed, 130 insertions(+), 8 deletions(-) diff --git a/Sources/SwiftIfConfig/ActiveSyntaxRewriter.swift b/Sources/SwiftIfConfig/ActiveSyntaxRewriter.swift index 0cdeb97e4e7..1ee7aa91dea 100644 --- a/Sources/SwiftIfConfig/ActiveSyntaxRewriter.swift +++ b/Sources/SwiftIfConfig/ActiveSyntaxRewriter.swift @@ -18,15 +18,20 @@ extension SyntaxProtocol { /// are inactive according to the given build configuration, leaving only /// the code that is active within that build configuration. /// - /// Returns the syntax node with all inactive regions removed, along with an - /// array containing any diagnostics produced along the way. - /// /// If there are errors in the conditions of any configuration /// clauses, e.g., `#if FOO > 10`, then the condition will be /// considered to have failed and the clauses's elements will be /// removed. + /// - Parameters: + /// - configuration: the configuration to apply. + /// - retainFeatureCheckIfConfigs: whether to retain `#if` blocks involving + /// compiler version checks (e.g., `compiler(>=6.0)`) and `$`-based + /// feature checks. + /// - Returns: the syntax node with all inactive regions removed, along with + /// an array containing any diagnostics produced along the way. public func removingInactive( - in configuration: some BuildConfiguration + in configuration: some BuildConfiguration, + retainFeatureCheckIfConfigs: Bool = false ) -> (result: Syntax, diagnostics: [Diagnostic]) { // First pass: Find all of the active clauses for the #ifs we need to // visit, along with any diagnostics produced along the way. This process @@ -41,7 +46,10 @@ extension SyntaxProtocol { // Second pass: Rewrite the syntax tree by removing the inactive clauses // from each #if (along with the #ifs themselves). - let rewriter = ActiveSyntaxRewriter(configuration: configuration) + let rewriter = ActiveSyntaxRewriter( + configuration: configuration, + retainFeatureCheckIfConfigs: retainFeatureCheckIfConfigs + ) return ( rewriter.rewrite(Syntax(self)), visitor.diagnostics @@ -83,8 +91,12 @@ class ActiveSyntaxRewriter: SyntaxRewriter { let configuration: Configuration var diagnostics: [Diagnostic] = [] - init(configuration: Configuration) { + /// Whether to retain `#if` blocks containing compiler and feature checks. + var retainFeatureCheckIfConfigs: Bool + + init(configuration: Configuration, retainFeatureCheckIfConfigs: Bool) { self.configuration = configuration + self.retainFeatureCheckIfConfigs = retainFeatureCheckIfConfigs } private func dropInactive( @@ -97,7 +109,9 @@ class ActiveSyntaxRewriter: SyntaxRewriter { let element = node[elementIndex] // Find #ifs within the list. - if let ifConfigDecl = elementAsIfConfig(element) { + if let ifConfigDecl = elementAsIfConfig(element), + (!retainFeatureCheckIfConfigs || !ifConfigDecl.containsFeatureCheck) + { // Retrieve the active `#if` clause let (activeClause, localDiagnostics) = ifConfigDecl.activeClause(in: configuration) @@ -262,6 +276,12 @@ class ActiveSyntaxRewriter: SyntaxRewriter { outerBase: ExprSyntax?, postfixIfConfig: PostfixIfConfigExprSyntax ) -> ExprSyntax { + // If we're supposed to retain #if configs that are feature checks, and + // this configuration has one, do so. + if retainFeatureCheckIfConfigs && postfixIfConfig.config.containsFeatureCheck { + return ExprSyntax(postfixIfConfig) + } + // Retrieve the active `if` clause. let (activeClause, localDiagnostics) = postfixIfConfig.config.activeClause(in: configuration) @@ -307,3 +327,55 @@ class ActiveSyntaxRewriter: SyntaxRewriter { return visit(rewrittenNode) } } + +/// Helper class to find a feature or compiler check. +fileprivate class FindFeatureCheckVisitor: SyntaxVisitor { + var foundFeatureCheck = false + + override func visit(_ node: DeclReferenceExprSyntax) -> SyntaxVisitorContinueKind { + // Checks that start with $ are feature checks that should be retained. + if let identifier = node.simpleIdentifier, + let initialChar = identifier.name.first, + initialChar == "$" + { + foundFeatureCheck = true + return .skipChildren + } + + return .visitChildren + } + + override func visit(_ node: FunctionCallExprSyntax) -> SyntaxVisitorContinueKind { + if let calleeDeclRef = node.calledExpression.as(DeclReferenceExprSyntax.self), + let calleeName = calleeDeclRef.simpleIdentifier?.name, + (calleeName == "compiler" || calleeName == "_compiler_version") + { + foundFeatureCheck = true + } + + return .skipChildren + } +} + +extension ExprSyntaxProtocol { + /// Whether any of the nodes in this expression involve compiler or feature + /// checks. + fileprivate var containsFeatureCheck: Bool { + let visitor = FindFeatureCheckVisitor(viewMode: .fixedUp) + visitor.walk(self) + return visitor.foundFeatureCheck + } +} + +extension IfConfigDeclSyntax { + /// Whether any of the clauses in this #if contain a feature check. + var containsFeatureCheck: Bool { + return clauses.contains { clause in + if let condition = clause.condition { + return condition.containsFeatureCheck + } else { + return false + } + } + } +} diff --git a/Tests/SwiftIfConfigTest/VisitorTests.swift b/Tests/SwiftIfConfigTest/VisitorTests.swift index 4e669e2b33a..f03246ba169 100644 --- a/Tests/SwiftIfConfigTest/VisitorTests.swift +++ b/Tests/SwiftIfConfigTest/VisitorTests.swift @@ -253,6 +253,52 @@ public class VisitorTests: XCTestCase { """ ) } + + func testRemoveInactiveRetainingFeatureChecks() { + assertRemoveInactive( + """ + public func hasIfCompilerCheck(_ x: () -> Bool = { + #if compiler(>=5.3) + return true + #else + return false + #endif + + #if $Blah + return 0 + #else + return 1 + #endif + + #if NOT_SET + return 3.14159 + #else + return 2.71828 + #endif + }) { + } + """, + configuration: linuxBuildConfig, + retainFeatureCheckIfConfigs: true, + expectedSource: """ + public func hasIfCompilerCheck(_ x: () -> Bool = { + #if compiler(>=5.3) + return true + #else + return false + #endif + + #if $Blah + return 0 + #else + return 1 + #endif + return 2.71828 + }) { + } + """ + ) + } } /// Assert that applying the given build configuration to the source code @@ -260,6 +306,7 @@ public class VisitorTests: XCTestCase { fileprivate func assertRemoveInactive( _ source: String, configuration: some BuildConfiguration, + retainFeatureCheckIfConfigs: Bool = false, diagnostics expectedDiagnostics: [DiagnosticSpec] = [], expectedSource: String, file: StaticString = #filePath, @@ -268,7 +315,10 @@ fileprivate func assertRemoveInactive( var parser = Parser(source) let tree = SourceFileSyntax.parse(from: &parser) - let (treeWithoutInactive, actualDiagnostics) = tree.removingInactive(in: configuration) + let (treeWithoutInactive, actualDiagnostics) = tree.removingInactive( + in: configuration, + retainFeatureCheckIfConfigs: retainFeatureCheckIfConfigs + ) // Check the resulting tree. assertStringsEqualWithDiff( From 392167b295432f0f2c4272076bdadbc6562d9cd5 Mon Sep 17 00:00:00 2001 From: Doug Gregor Date: Fri, 30 Aug 2024 16:02:55 -0700 Subject: [PATCH 2/7] Add a compiler SPI for "description without comments" Introduce a new `SyntaxProtocol.descriptionWithoutComments` that removes comments from the given syntax, replacing them with either a space (if the comment has no newlines) or a newline (if it does have newlines). --- .../SwiftIfConfig/ActiveSyntaxRewriter.swift | 35 +++++++++++++++++ Tests/SwiftIfConfigTest/VisitorTests.swift | 38 ++++++++++++++++++- 2 files changed, 72 insertions(+), 1 deletion(-) diff --git a/Sources/SwiftIfConfig/ActiveSyntaxRewriter.swift b/Sources/SwiftIfConfig/ActiveSyntaxRewriter.swift index 1ee7aa91dea..fef938a2304 100644 --- a/Sources/SwiftIfConfig/ActiveSyntaxRewriter.swift +++ b/Sources/SwiftIfConfig/ActiveSyntaxRewriter.swift @@ -379,3 +379,38 @@ extension IfConfigDeclSyntax { } } } + +extension SyntaxProtocol { + // Produce the source code for this syntax node with all of the comments + // removed. Each comment will be replaced with either a newline or a space, + // depending on whether the comment involved a newline. + @_spi(Compiler) + public var descriptionWithoutComments: String { + var result = "" + for token in tokens(viewMode: .sourceAccurate) { + token.leadingTrivia.writeWithoutComments(to: &result) + token.text.write(to: &result) + token.trailingTrivia.writeWithoutComments(to: &result) + } + return result + } +} + +extension Trivia { + fileprivate func writeWithoutComments(to stream: inout some TextOutputStream) { + for piece in pieces { + switch piece { + case .backslashes, .carriageReturnLineFeeds, .carriageReturns, .formfeeds, .newlines, .pounds, .spaces, .tabs, + .unexpectedText, .verticalTabs: + piece.write(to: &stream) + + case .blockComment(let text), .docBlockComment(let text), .docLineComment(let text), .lineComment(let text): + if text.contains(where: \.isNewline) { + stream.write("\n") + } else { + stream.write(" ") + } + } + } + } +} diff --git a/Tests/SwiftIfConfigTest/VisitorTests.swift b/Tests/SwiftIfConfigTest/VisitorTests.swift index f03246ba169..df1687e1faf 100644 --- a/Tests/SwiftIfConfigTest/VisitorTests.swift +++ b/Tests/SwiftIfConfigTest/VisitorTests.swift @@ -11,7 +11,7 @@ //===----------------------------------------------------------------------===// import SwiftDiagnostics -import SwiftIfConfig +@_spi(Compiler) import SwiftIfConfig import SwiftParser import SwiftSyntax @_spi(XCTestFailureLocation) @_spi(Testing) import SwiftSyntaxMacrosGenericTestSupport @@ -299,6 +299,42 @@ public class VisitorTests: XCTestCase { """ ) } + + func testRemoveComments() { + let original: SourceFileSyntax = """ + + /// This is a documentation comment + func f() { } + + /** Another documentation comment + that is split across + multiple lines */ + func g() { } + + func h() { + x +/*comment*/y + // foo + } + """ + + assertStringsEqualWithDiff( + original.descriptionWithoutComments, + """ + + + func f() { } + + + + func g() { } + + func h() { + x + y + + } + """ + ) + } } /// Assert that applying the given build configuration to the source code From 7036a41215d54edc9f07d2277dc865570a1a6a13 Mon Sep 17 00:00:00 2001 From: Doug Gregor Date: Fri, 30 Aug 2024 16:41:48 -0700 Subject: [PATCH 3/7] Expand `descriptionWithoutComments` to also remove #sourceLocation --- .../SwiftIfConfig/ActiveSyntaxRewriter.swift | 20 ++++++++++++++++--- Tests/SwiftIfConfigTest/VisitorTests.swift | 6 +++--- 2 files changed, 20 insertions(+), 6 deletions(-) diff --git a/Sources/SwiftIfConfig/ActiveSyntaxRewriter.swift b/Sources/SwiftIfConfig/ActiveSyntaxRewriter.swift index fef938a2304..e0e6b2b095f 100644 --- a/Sources/SwiftIfConfig/ActiveSyntaxRewriter.swift +++ b/Sources/SwiftIfConfig/ActiveSyntaxRewriter.swift @@ -382,12 +382,26 @@ extension IfConfigDeclSyntax { extension SyntaxProtocol { // Produce the source code for this syntax node with all of the comments - // removed. Each comment will be replaced with either a newline or a space, - // depending on whether the comment involved a newline. + // and #sourceLocations removed. Each comment will be replaced with either + // a newline or a space, depending on whether the comment involved a newline. @_spi(Compiler) - public var descriptionWithoutComments: String { + public var descriptionWithoutCommentsAndSourceLocations: String { var result = "" + var skipUntilRParen = false for token in tokens(viewMode: .sourceAccurate) { + // Skip #sourceLocation(...). + if token.tokenKind == .poundSourceLocation { + skipUntilRParen = true + continue + } + + if skipUntilRParen { + if token.tokenKind == .rightParen { + skipUntilRParen = false + } + continue + } + token.leadingTrivia.writeWithoutComments(to: &result) token.text.write(to: &result) token.trailingTrivia.writeWithoutComments(to: &result) diff --git a/Tests/SwiftIfConfigTest/VisitorTests.swift b/Tests/SwiftIfConfigTest/VisitorTests.swift index df1687e1faf..e476971dd30 100644 --- a/Tests/SwiftIfConfigTest/VisitorTests.swift +++ b/Tests/SwiftIfConfigTest/VisitorTests.swift @@ -300,12 +300,13 @@ public class VisitorTests: XCTestCase { ) } - func testRemoveComments() { + func testRemoveCommentsAndSourceLocations() { let original: SourceFileSyntax = """ /// This is a documentation comment func f() { } + #sourceLocation(file: "if-configs.swift", line: 200) /** Another documentation comment that is split across multiple lines */ @@ -318,14 +319,13 @@ public class VisitorTests: XCTestCase { """ assertStringsEqualWithDiff( - original.descriptionWithoutComments, + original.descriptionWithoutCommentsAndSourceLocations, """ func f() { } - func g() { } func h() { From 2115d772132f775dd0d571439ef6b4328c7abd88 Mon Sep 17 00:00:00 2001 From: Doug Gregor Date: Mon, 2 Sep 2024 22:08:43 -0700 Subject: [PATCH 4/7] Make the `retainFeatureCheckIfConfigs` version of `removingInactive` compiler SPI I'm not sure we have any clients for this particular option outside of the compiler, so for now, limit it to the compiler. We can expose it later if we'd like. --- .../SwiftIfConfig/ActiveSyntaxRewriter.swift | 21 ++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-) diff --git a/Sources/SwiftIfConfig/ActiveSyntaxRewriter.swift b/Sources/SwiftIfConfig/ActiveSyntaxRewriter.swift index e0e6b2b095f..edf7528454f 100644 --- a/Sources/SwiftIfConfig/ActiveSyntaxRewriter.swift +++ b/Sources/SwiftIfConfig/ActiveSyntaxRewriter.swift @@ -14,6 +14,24 @@ import SwiftDiagnostics import SwiftSyntax extension SyntaxProtocol { + /// Produce a copy of this syntax node that removes all syntax regions that + /// are inactive according to the given build configuration, leaving only + /// the code that is active within that build configuration. + /// + /// If there are errors in the conditions of any configuration + /// clauses, e.g., `#if FOO > 10`, then the condition will be + /// considered to have failed and the clauses's elements will be + /// removed. + /// - Parameters: + /// - configuration: the configuration to apply. + /// - Returns: the syntax node with all inactive regions removed, along with + /// an array containing any diagnostics produced along the way. + public func removingInactive( + in configuration: some BuildConfiguration + ) -> (result: Syntax, diagnostics: [Diagnostic]) { + return removingInactive(in: configuration, retainFeatureCheckIfConfigs: false) + } + /// Produce a copy of this syntax node that removes all syntax regions that /// are inactive according to the given build configuration, leaving only /// the code that is active within that build configuration. @@ -29,9 +47,10 @@ extension SyntaxProtocol { /// feature checks. /// - Returns: the syntax node with all inactive regions removed, along with /// an array containing any diagnostics produced along the way. + @_spi(Compiler) public func removingInactive( in configuration: some BuildConfiguration, - retainFeatureCheckIfConfigs: Bool = false + retainFeatureCheckIfConfigs: Bool ) -> (result: Syntax, diagnostics: [Diagnostic]) { // First pass: Find all of the active clauses for the #ifs we need to // visit, along with any diagnostics produced along the way. This process From 5fba64644c14d0b1cfe8fb86f76f410c10ec12a5 Mon Sep 17 00:00:00 2001 From: Doug Gregor Date: Mon, 2 Sep 2024 23:15:58 -0700 Subject: [PATCH 5/7] Rework ActiveSyntaxRewriter to operate on top of ConfiguredRegions The operation that removes all inactive (and unparsed) regions from a syntax tree is a two-pass algorithm. For clients that have already created a ConfiguredRegions instance, let that serve as the first pass. ConfiguredRegions now tracks the active clauses of any evaluated `#if`s, and the ActiveSyntaxRewriter uses that information to guide its traversal and rewrite rather than re-evaluating all of the clauses. This required reworking the way in which we handle recursion in ActiveSyntaxRewriter. Before, we were rewriting nodes "outside-in", but doing so broke node identity and, therefore, lookup of the active #if clauses. Instead, make sure we rewrite from the leaves up to the root. Clients still have the option of removing inactive/unparsed code from a syntax node given just a build configuration, in which case we'll first create a ConfiguredRegions. --- .../SwiftIfConfig/ActiveSyntaxRewriter.swift | 146 +++++++++++------- .../SwiftIfConfig/ActiveSyntaxVisitor.swift | 5 - Sources/SwiftIfConfig/ConfiguredRegions.swift | 29 ++++ 3 files changed, 120 insertions(+), 60 deletions(-) diff --git a/Sources/SwiftIfConfig/ActiveSyntaxRewriter.swift b/Sources/SwiftIfConfig/ActiveSyntaxRewriter.swift index edf7528454f..78cf2de249f 100644 --- a/Sources/SwiftIfConfig/ActiveSyntaxRewriter.swift +++ b/Sources/SwiftIfConfig/ActiveSyntaxRewriter.swift @@ -52,27 +52,64 @@ extension SyntaxProtocol { in configuration: some BuildConfiguration, retainFeatureCheckIfConfigs: Bool ) -> (result: Syntax, diagnostics: [Diagnostic]) { - // First pass: Find all of the active clauses for the #ifs we need to - // visit, along with any diagnostics produced along the way. This process - // does not change the tree in any way. - let visitor = ActiveSyntaxVisitor(viewMode: .sourceAccurate, configuration: configuration) - visitor.walk(self) + let configuredRegions = configuredRegions(in: configuration) + return ( + result: configuredRegions.removingInactive( + from: self, + retainFeatureCheckIfConfigs: retainFeatureCheckIfConfigs + ), + diagnostics: configuredRegions.diagnostics + ) + } +} - // If there were no active clauses to visit, we're done! - if !visitor.visitedAnyIfClauses { - return (Syntax(self), visitor.diagnostics) +extension ConfiguredRegions { + /// Produce a copy of some syntax node in the configured region that removes + /// all syntax regions that are inactive according to the build configuration, + /// leaving only the code that is active within that build configuration. + /// + /// If there are errors in the conditions of any configuration + /// clauses, e.g., `#if FOO > 10`, then the condition will be + /// considered to have failed and the clauses's elements will be + /// removed. + /// - Parameters: + /// - node: the stnrax node from which inactive regions will be removed. + /// - Returns: the syntax node with all inactive regions removed. + public func removingInactive(from node: some SyntaxProtocol) -> Syntax { + return removingInactive(from: node, retainFeatureCheckIfConfigs: false) + } + + /// Produce a copy of some syntax node in the configured region that removes + /// all syntax regions that are inactive according to the build configuration, + /// leaving only the code that is active within that build configuration. + /// + /// If there are errors in the conditions of any configuration + /// clauses, e.g., `#if FOO > 10`, then the condition will be + /// considered to have failed and the clauses's elements will be + /// removed. + /// - Parameters: + /// - node: the stnrax node from which inactive regions will be removed. + /// - retainFeatureCheckIfConfigs: whether to retain `#if` blocks involving + /// compiler version checks (e.g., `compiler(>=6.0)`) and `$`-based + /// feature checks. + /// - Returns: the syntax node with all inactive regions removed. + @_spi(Compiler) + public func removingInactive( + from node: some SyntaxProtocol, + retainFeatureCheckIfConfigs: Bool + ) -> Syntax { + // If there are no inactive regions, there's nothing to do. + if regions.isEmpty { + return Syntax(node) } - // Second pass: Rewrite the syntax tree by removing the inactive clauses + // Rewrite the syntax tree by removing the inactive clauses // from each #if (along with the #ifs themselves). let rewriter = ActiveSyntaxRewriter( - configuration: configuration, + configuredRegions: self, retainFeatureCheckIfConfigs: retainFeatureCheckIfConfigs ) - return ( - rewriter.rewrite(Syntax(self)), - visitor.diagnostics - ) + return rewriter.rewrite(Syntax(node)) } } @@ -106,15 +143,16 @@ extension SyntaxProtocol { /// /// For any other target platforms, the resulting tree will be empty (other /// than trivia). -class ActiveSyntaxRewriter: SyntaxRewriter { - let configuration: Configuration - var diagnostics: [Diagnostic] = [] +class ActiveSyntaxRewriter: SyntaxRewriter { + let configuredRegions: ConfiguredRegions + var diagnostics: [Diagnostic] /// Whether to retain `#if` blocks containing compiler and feature checks. var retainFeatureCheckIfConfigs: Bool - init(configuration: Configuration, retainFeatureCheckIfConfigs: Bool) { - self.configuration = configuration + init(configuredRegions: ConfiguredRegions, retainFeatureCheckIfConfigs: Bool) { + self.configuredRegions = configuredRegions + self.diagnostics = configuredRegions.diagnostics self.retainFeatureCheckIfConfigs = retainFeatureCheckIfConfigs } @@ -124,6 +162,19 @@ class ActiveSyntaxRewriter: SyntaxRewriter { ) -> List { var newElements: [List.Element] = [] var anyChanged = false + + // Note that an element changed at the given index. + func noteElementChanged(at elementIndex: List.Index) { + if anyChanged { + return + } + + // This is the first element that changed, so note that we have + // changes and add all prior elements to the list of new elements. + anyChanged = true + newElements.append(contentsOf: node[..: SyntaxRewriter { (!retainFeatureCheckIfConfigs || !ifConfigDecl.containsFeatureCheck) { // Retrieve the active `#if` clause - let (activeClause, localDiagnostics) = ifConfigDecl.activeClause(in: configuration) - - // Add these diagnostics. - diagnostics.append(contentsOf: localDiagnostics) + let activeClause = configuredRegions.activeClause(for: ifConfigDecl) - // If this is the first element that changed, note that we have - // changes and add all prior elements to the list of new elements. - if !anyChanged { - anyChanged = true - newElements.append(contentsOf: node[..: SyntaxRewriter { continue } + // Transform the element directly. If it changed, note the changes. + if let transformedElement = rewrite(Syntax(element)).as(List.Element.self), + transformedElement.id != element.id + { + noteElementChanged(at: elementIndex) + newElements.append(transformedElement) + continue + } + if anyChanged { newElements.append(element) } @@ -174,47 +226,39 @@ class ActiveSyntaxRewriter: SyntaxRewriter { } override func visit(_ node: CodeBlockItemListSyntax) -> CodeBlockItemListSyntax { - let rewrittenNode = dropInactive(node) { element in + return dropInactive(node) { element in guard case .decl(let declElement) = element.item else { return nil } return declElement.as(IfConfigDeclSyntax.self) } - - return super.visit(rewrittenNode) } override func visit(_ node: MemberBlockItemListSyntax) -> MemberBlockItemListSyntax { - let rewrittenNode = dropInactive(node) { element in + return dropInactive(node) { element in return element.decl.as(IfConfigDeclSyntax.self) } - - return super.visit(rewrittenNode) } override func visit(_ node: SwitchCaseListSyntax) -> SwitchCaseListSyntax { - let rewrittenNode = dropInactive(node) { element in + return dropInactive(node) { element in if case .ifConfigDecl(let ifConfigDecl) = element { return ifConfigDecl } return nil } - - return super.visit(rewrittenNode) } override func visit(_ node: AttributeListSyntax) -> AttributeListSyntax { - let rewrittenNode = dropInactive(node) { element in + return dropInactive(node) { element in if case .ifConfigDecl(let ifConfigDecl) = element { return ifConfigDecl } return nil } - - return super.visit(rewrittenNode) } /// Apply the given base to the postfix expression. @@ -234,7 +278,7 @@ class ActiveSyntaxRewriter: SyntaxRewriter { return nil } - let newExpr = applyBaseToPostfixExpression(base: base, postfix: node[keyPath: keyPath]) + let newExpr = applyBaseToPostfixExpression(base: base, postfix: visit(node[keyPath: keyPath])) return ExprSyntax(node.with(keyPath, newExpr)) } @@ -243,7 +287,7 @@ class ActiveSyntaxRewriter: SyntaxRewriter { guard let memberBase = memberAccess.base else { // If this member access has no base, this is the base we are // replacing, terminating the recursion. Do so now. - return ExprSyntax(memberAccess.with(\.base, base)) + return ExprSyntax(memberAccess.with(\.base, visit(base))) } let newBase = applyBaseToPostfixExpression(base: base, postfix: memberBase) @@ -302,10 +346,7 @@ class ActiveSyntaxRewriter: SyntaxRewriter { } // Retrieve the active `if` clause. - let (activeClause, localDiagnostics) = postfixIfConfig.config.activeClause(in: configuration) - - // Record these diagnostics. - diagnostics.append(contentsOf: localDiagnostics) + let activeClause = configuredRegions.activeClause(for: postfixIfConfig.config) guard case .postfixExpression(let postfixExpr) = activeClause?.elements else { @@ -315,7 +356,7 @@ class ActiveSyntaxRewriter: SyntaxRewriter { // only have both in an ill-formed syntax tree that was manually // created. if let base = postfixIfConfig.base ?? outerBase { - return base + return visit(base) } // If there was no base, we're in an erroneous syntax tree that would @@ -330,7 +371,7 @@ class ActiveSyntaxRewriter: SyntaxRewriter { // If there is no base, return the postfix expression. guard let base = postfixIfConfig.base ?? outerBase else { - return postfixExpr + return visit(postfixExpr) } // Apply the base to the postfix expression. @@ -338,12 +379,7 @@ class ActiveSyntaxRewriter: SyntaxRewriter { } override func visit(_ node: PostfixIfConfigExprSyntax) -> ExprSyntax { - let rewrittenNode = dropInactive(outerBase: nil, postfixIfConfig: node) - if rewrittenNode == ExprSyntax(node) { - return rewrittenNode - } - - return visit(rewrittenNode) + return dropInactive(outerBase: nil, postfixIfConfig: node) } } diff --git a/Sources/SwiftIfConfig/ActiveSyntaxVisitor.swift b/Sources/SwiftIfConfig/ActiveSyntaxVisitor.swift index e202f11809f..65a1e9c9c7f 100644 --- a/Sources/SwiftIfConfig/ActiveSyntaxVisitor.swift +++ b/Sources/SwiftIfConfig/ActiveSyntaxVisitor.swift @@ -46,9 +46,6 @@ open class ActiveSyntaxVisitor: SyntaxVisitor /// The diagnostics accumulated during this walk of active syntax. public private(set) var diagnostics: [Diagnostic] = [] - /// Whether we visited any "#if" clauses. - var visitedAnyIfClauses: Bool = false - public init(viewMode: SyntaxTreeViewMode, configuration: Configuration) { self.configuration = configuration super.init(viewMode: viewMode) @@ -60,8 +57,6 @@ open class ActiveSyntaxVisitor: SyntaxVisitor let (activeClause, localDiagnostics) = node.activeClause(in: configuration) diagnostics.append(contentsOf: localDiagnostics) - visitedAnyIfClauses = true - // If there is an active clause, visit it's children. if let activeClause, let elements = activeClause.elements { walk(elements) diff --git a/Sources/SwiftIfConfig/ConfiguredRegions.swift b/Sources/SwiftIfConfig/ConfiguredRegions.swift index bd83252c399..67d1afb29c6 100644 --- a/Sources/SwiftIfConfig/ConfiguredRegions.swift +++ b/Sources/SwiftIfConfig/ConfiguredRegions.swift @@ -39,6 +39,12 @@ import SwiftSyntax public struct ConfiguredRegions { let regions: [(ifClause: IfConfigClauseSyntax, state: IfConfigRegionState)] + // A mapping from each of the #if declarations that have been evaluated to + // the active clause. Absence from this map means that there is no active + // clause, either because every clause failed or because the entire #if + // itself is inactive. + var activeClauses: [IfConfigDeclSyntax: IfConfigClauseSyntax] + /// The set of diagnostics produced when evaluating the configured regions. public let diagnostics: [Diagnostic] @@ -80,6 +86,17 @@ public struct ConfiguredRegions { (region.ifClause.regionStart...region.ifClause.endPosition).contains(node.position) }?.state ?? .active } + + /// Determine which clause of an `#if` declaration was active within this + /// set of configured regions. + /// + /// A particular `#if` declaration might have no active clause (e.g., this + /// operation will return a `nil`) if either none of the clauses had + /// conditions that succeeded, or the `#if` declaration itself is within an + /// inactive (or unparsed) region and therefore cannot have an active clause. + public func activeClause(for node: IfConfigDeclSyntax) -> IfConfigClauseSyntax? { + return activeClauses[node] + } } extension ConfiguredRegions: RandomAccessCollection { @@ -150,6 +167,7 @@ extension SyntaxProtocol { visitor.walk(self) return ConfiguredRegions( regions: visitor.regions, + activeClauses: visitor.activeClauses, diagnostics: visitor.diagnostics ) } @@ -171,6 +189,12 @@ fileprivate class ConfiguredRegionVisitor: Sy // All diagnostics encountered along the way. var diagnostics: [Diagnostic] = [] + // A mapping from each of the #if declarations that have been evaluated to + // the active clause. Absence from this map means that there is no active + // clause, either because every clause failed or because the entire #if + // itself is inactive. + var activeClauses: [IfConfigDeclSyntax: IfConfigClauseSyntax] = [:] + init(configuration: Configuration) { self.configuration = configuration super.init(viewMode: .sourceAccurate) @@ -242,6 +266,11 @@ fileprivate class ConfiguredRegionVisitor: Sy isActive = !foundActive && inActiveRegion } + // If this is the active clause, record it as such. + if isActive { + activeClauses[node] = clause + } + // Determine and record the current state. let currentState: IfConfigRegionState switch (isActive, syntaxErrorsAllowed) { From d50749fc10371a24548172ee04ad02f227917dfc Mon Sep 17 00:00:00 2001 From: Doug Gregor Date: Mon, 2 Sep 2024 23:46:42 -0700 Subject: [PATCH 6/7] Teach the ActiveSyntaxRewriter to use either ConfiguredRegions or BuildConfiguration Instead of always forcing the construction of ConfiguredRegions ahead of time, introduce the notion of an ActiveClauseEvaluator that stores either configured regions or a build configuration, and can query for the active clause within a given #if declaration from either representation. Use this in ActiveSyntaxRewriter to make the operation itself single-pass, always. Always test both paths to make sure they stay in sync. --- .../SwiftIfConfig/ActiveClauseEvaluator.swift | 50 ++++++++++++ .../SwiftIfConfig/ActiveSyntaxRewriter.swift | 30 ++++--- Sources/SwiftIfConfig/CMakeLists.txt | 1 + Tests/SwiftIfConfigTest/VisitorTests.swift | 80 ++++++++++++------- 4 files changed, 120 insertions(+), 41 deletions(-) create mode 100644 Sources/SwiftIfConfig/ActiveClauseEvaluator.swift diff --git a/Sources/SwiftIfConfig/ActiveClauseEvaluator.swift b/Sources/SwiftIfConfig/ActiveClauseEvaluator.swift new file mode 100644 index 00000000000..8e7b9e2f5d9 --- /dev/null +++ b/Sources/SwiftIfConfig/ActiveClauseEvaluator.swift @@ -0,0 +1,50 @@ +//===----------------------------------------------------------------------===// +// +// This source file is part of the Swift.org open source project +// +// Copyright (c) 2014 - 2023 Apple Inc. and the Swift project authors +// Licensed under Apache License v2.0 with Runtime Library Exception +// +// See https://swift.org/LICENSE.txt for license information +// See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors +// +//===----------------------------------------------------------------------===// + +import SwiftDiagnostics +import SwiftSyntax + +/// Captures sufficient information to determine the active clause for an `#if` +/// either by querying existing configured regions or by evaluating the +/// clause's conditions against a build configuration. +enum ActiveClauseEvaluator { + case configuredRegions(ConfiguredRegions) + case configuration(any BuildConfiguration) + + /// Previously-known diagnostics. + var priorDiagnostics: [Diagnostic] { + switch self { + case .configuredRegions(let configuredRegions): + return configuredRegions.diagnostics + case .configuration: + return [] + } + } + + /// Determine which clause of an `#if` declaration is active, if any. + /// + /// If this evaluation produced any diagnostics, they will be appended to + /// the diagnostics parameter. + func activeClause( + for node: IfConfigDeclSyntax, + diagnostics: inout [Diagnostic] + ) -> IfConfigClauseSyntax? { + switch self { + case .configuredRegions(let configuredRegions): + return configuredRegions.activeClause(for: node) + case .configuration(let configuration): + let (activeClause, localDiagnostics) = node.activeClause(in: configuration) + diagnostics.append(contentsOf: localDiagnostics) + return activeClause + } + } +} diff --git a/Sources/SwiftIfConfig/ActiveSyntaxRewriter.swift b/Sources/SwiftIfConfig/ActiveSyntaxRewriter.swift index 78cf2de249f..f4ac3b4c695 100644 --- a/Sources/SwiftIfConfig/ActiveSyntaxRewriter.swift +++ b/Sources/SwiftIfConfig/ActiveSyntaxRewriter.swift @@ -52,13 +52,15 @@ extension SyntaxProtocol { in configuration: some BuildConfiguration, retainFeatureCheckIfConfigs: Bool ) -> (result: Syntax, diagnostics: [Diagnostic]) { - let configuredRegions = configuredRegions(in: configuration) + // Rewrite the syntax tree by removing the inactive clauses + // from each #if (along with the #ifs themselves). + let rewriter = ActiveSyntaxRewriter( + configuration: configuration, + retainFeatureCheckIfConfigs: retainFeatureCheckIfConfigs + ) return ( - result: configuredRegions.removingInactive( - from: self, - retainFeatureCheckIfConfigs: retainFeatureCheckIfConfigs - ), - diagnostics: configuredRegions.diagnostics + rewriter.rewrite(Syntax(self)), + rewriter.diagnostics ) } } @@ -144,15 +146,21 @@ extension ConfiguredRegions { /// For any other target platforms, the resulting tree will be empty (other /// than trivia). class ActiveSyntaxRewriter: SyntaxRewriter { - let configuredRegions: ConfiguredRegions + let activeClauses: ActiveClauseEvaluator var diagnostics: [Diagnostic] /// Whether to retain `#if` blocks containing compiler and feature checks. var retainFeatureCheckIfConfigs: Bool init(configuredRegions: ConfiguredRegions, retainFeatureCheckIfConfigs: Bool) { - self.configuredRegions = configuredRegions - self.diagnostics = configuredRegions.diagnostics + self.activeClauses = .configuredRegions(configuredRegions) + self.diagnostics = activeClauses.priorDiagnostics + self.retainFeatureCheckIfConfigs = retainFeatureCheckIfConfigs + } + + init(configuration: some BuildConfiguration, retainFeatureCheckIfConfigs: Bool) { + self.activeClauses = .configuration(configuration) + self.diagnostics = activeClauses.priorDiagnostics self.retainFeatureCheckIfConfigs = retainFeatureCheckIfConfigs } @@ -183,7 +191,7 @@ class ActiveSyntaxRewriter: SyntaxRewriter { (!retainFeatureCheckIfConfigs || !ifConfigDecl.containsFeatureCheck) { // Retrieve the active `#if` clause - let activeClause = configuredRegions.activeClause(for: ifConfigDecl) + let activeClause = activeClauses.activeClause(for: ifConfigDecl, diagnostics: &diagnostics) noteElementChanged(at: elementIndex) @@ -346,7 +354,7 @@ class ActiveSyntaxRewriter: SyntaxRewriter { } // Retrieve the active `if` clause. - let activeClause = configuredRegions.activeClause(for: postfixIfConfig.config) + let activeClause = activeClauses.activeClause(for: postfixIfConfig.config, diagnostics: &diagnostics) guard case .postfixExpression(let postfixExpr) = activeClause?.elements else { diff --git a/Sources/SwiftIfConfig/CMakeLists.txt b/Sources/SwiftIfConfig/CMakeLists.txt index 422577e474b..2df83517c5b 100644 --- a/Sources/SwiftIfConfig/CMakeLists.txt +++ b/Sources/SwiftIfConfig/CMakeLists.txt @@ -7,6 +7,7 @@ # See http://swift.org/CONTRIBUTORS.txt for Swift project authors add_swift_syntax_library(SwiftIfConfig + ActiveClauseEvaluator.swift ActiveSyntaxVisitor.swift ActiveSyntaxRewriter.swift BuildConfiguration.swift diff --git a/Tests/SwiftIfConfigTest/VisitorTests.swift b/Tests/SwiftIfConfigTest/VisitorTests.swift index e476971dd30..b42a90e2412 100644 --- a/Tests/SwiftIfConfigTest/VisitorTests.swift +++ b/Tests/SwiftIfConfigTest/VisitorTests.swift @@ -337,8 +337,8 @@ public class VisitorTests: XCTestCase { } } -/// Assert that applying the given build configuration to the source code -/// returns the expected source and diagnostics. +/// Assert that removing any inactive code according to the given build +/// configuration returns the expected source and diagnostics. fileprivate func assertRemoveInactive( _ source: String, configuration: some BuildConfiguration, @@ -351,39 +351,59 @@ fileprivate func assertRemoveInactive( var parser = Parser(source) let tree = SourceFileSyntax.parse(from: &parser) - let (treeWithoutInactive, actualDiagnostics) = tree.removingInactive( - in: configuration, - retainFeatureCheckIfConfigs: retainFeatureCheckIfConfigs - ) - - // Check the resulting tree. - assertStringsEqualWithDiff( - treeWithoutInactive.description, - expectedSource, - file: file, - line: line - ) + for useConfiguredRegions in [false, true] { + let fromDescription = useConfiguredRegions ? "configured regions" : "build configuration" + let treeWithoutInactive: Syntax + let actualDiagnostics: [Diagnostic] + + if useConfiguredRegions { + let configuredRegions = tree.configuredRegions(in: configuration) + actualDiagnostics = configuredRegions.diagnostics + treeWithoutInactive = configuredRegions.removingInactive( + from: tree, + retainFeatureCheckIfConfigs: retainFeatureCheckIfConfigs + ) + } else { + (treeWithoutInactive, actualDiagnostics) = tree.removingInactive( + in: configuration, + retainFeatureCheckIfConfigs: retainFeatureCheckIfConfigs + ) + } - // Check the diagnostics. - if actualDiagnostics.count != expectedDiagnostics.count { - XCTFail( - """ - Expected \(expectedDiagnostics.count) diagnostics, but got \(actualDiagnostics.count): - \(actualDiagnostics.map(\.debugDescription).joined(separator: "\n")) - """, + // Check the resulting tree. + assertStringsEqualWithDiff( + treeWithoutInactive.description, + expectedSource, + "Active code (\(fromDescription))", file: file, line: line ) - } else { - for (actualDiag, expectedDiag) in zip(actualDiagnostics, expectedDiagnostics) { - assertDiagnostic( - actualDiag, - in: .tree(tree), - expected: expectedDiag, - failureHandler: { - XCTFail($0.message, file: $0.location.staticFilePath, line: $0.location.unsignedLine) - } + + // Check the diagnostics. + if actualDiagnostics.count != expectedDiagnostics.count { + XCTFail( + """ + Expected \(expectedDiagnostics.count) diagnostics, but got \(actualDiagnostics.count) via \(fromDescription): + \(actualDiagnostics.map(\.debugDescription).joined(separator: "\n")) + """, + file: file, + line: line ) + } else { + for (actualDiag, expectedDiag) in zip(actualDiagnostics, expectedDiagnostics) { + assertDiagnostic( + actualDiag, + in: .tree(tree), + expected: expectedDiag, + failureHandler: { + XCTFail( + $0.message + " via \(fromDescription)", + file: $0.location.staticFilePath, + line: $0.location.unsignedLine + ) + } + ) + } } } } From 149ef642258d392914b44b63a619735e6849dd46 Mon Sep 17 00:00:00 2001 From: Doug Gregor Date: Tue, 3 Sep 2024 00:07:01 -0700 Subject: [PATCH 7/7] Teach ActiveSyntax(Any)Visitor to use configured regions or a build configuration Rework the implementations of ActiveSyntax(Any)Visitor to allow configured regions, so that clients can re-use the work of evaluating the build configuration for later traversals. As a side effect of this, ActiveSyntax(Any)Visitor are no longer generic over the type of build configuration, which should make them easier to use. --- .../SwiftIfConfig/ActiveSyntaxVisitor.swift | 42 +++++--- .../SwiftIfConfig.docc/SwiftIfConfig.md | 3 +- Tests/SwiftIfConfigTest/VisitorTests.swift | 98 +++++++++++++++---- 3 files changed, 109 insertions(+), 34 deletions(-) diff --git a/Sources/SwiftIfConfig/ActiveSyntaxVisitor.swift b/Sources/SwiftIfConfig/ActiveSyntaxVisitor.swift index 65a1e9c9c7f..ba1d3ccfe46 100644 --- a/Sources/SwiftIfConfig/ActiveSyntaxVisitor.swift +++ b/Sources/SwiftIfConfig/ActiveSyntaxVisitor.swift @@ -39,23 +39,30 @@ import SwiftSyntax /// All notes visited by this visitor will have the "active" state, i.e., /// `node.isActive(in: configuration)` will have evaluated to `.active`. /// When errors occur, they will be recorded in the array of diagnostics. -open class ActiveSyntaxVisitor: SyntaxVisitor { - /// The build configuration, which will be queried for each relevant `#if`. - public let configuration: Configuration +open class ActiveSyntaxVisitor: SyntaxVisitor { + /// The abstracted build configuration, which will be queried for each + /// relevant `#if`. + let activeClauses: ActiveClauseEvaluator /// The diagnostics accumulated during this walk of active syntax. public private(set) var diagnostics: [Diagnostic] = [] - public init(viewMode: SyntaxTreeViewMode, configuration: Configuration) { - self.configuration = configuration + public init(viewMode: SyntaxTreeViewMode, configuration: some BuildConfiguration) { + self.activeClauses = .configuration(configuration) + self.diagnostics = activeClauses.priorDiagnostics + super.init(viewMode: viewMode) + } + + public init(viewMode: SyntaxTreeViewMode, configuredRegions: ConfiguredRegions) { + self.activeClauses = .configuredRegions(configuredRegions) + self.diagnostics = activeClauses.priorDiagnostics super.init(viewMode: viewMode) } open override func visit(_ node: IfConfigDeclSyntax) -> SyntaxVisitorContinueKind { // Note: there is a clone of this code in ActiveSyntaxAnyVisitor. If you // change one, please also change the other. - let (activeClause, localDiagnostics) = node.activeClause(in: configuration) - diagnostics.append(contentsOf: localDiagnostics) + let activeClause = activeClauses.activeClause(for: node, diagnostics: &diagnostics) // If there is an active clause, visit it's children. if let activeClause, let elements = activeClause.elements { @@ -93,15 +100,23 @@ open class ActiveSyntaxVisitor: SyntaxVisitor /// All notes visited by this visitor will have the "active" state, i.e., /// `node.isActive(in: configuration)` will have evaluated to `.active`. /// When errors occur, they will be recorded in the array of diagnostics. -open class ActiveSyntaxAnyVisitor: SyntaxAnyVisitor { - /// The build configuration, which will be queried for each relevant `#if`. - public let configuration: Configuration +open class ActiveSyntaxAnyVisitor: SyntaxAnyVisitor { + /// The abstracted build configuration, which will be queried for each + /// relevant `#if`. + let activeClauses: ActiveClauseEvaluator /// The diagnostics accumulated during this walk of active syntax. public private(set) var diagnostics: [Diagnostic] = [] - public init(viewMode: SyntaxTreeViewMode, configuration: Configuration) { - self.configuration = configuration + public init(viewMode: SyntaxTreeViewMode, configuration: some BuildConfiguration) { + self.activeClauses = .configuration(configuration) + self.diagnostics = activeClauses.priorDiagnostics + super.init(viewMode: viewMode) + } + + public init(viewMode: SyntaxTreeViewMode, configuredRegions: ConfiguredRegions) { + self.activeClauses = .configuredRegions(configuredRegions) + self.diagnostics = activeClauses.priorDiagnostics super.init(viewMode: viewMode) } @@ -110,8 +125,7 @@ open class ActiveSyntaxAnyVisitor: SyntaxAnyV // change one, please also change the other. // If there is an active clause, visit it's children. - let (activeClause, localDiagnostics) = node.activeClause(in: configuration) - diagnostics.append(contentsOf: localDiagnostics) + let activeClause = activeClauses.activeClause(for: node, diagnostics: &diagnostics) if let activeClause, let elements = activeClause.elements { walk(elements) diff --git a/Sources/SwiftIfConfig/SwiftIfConfig.docc/SwiftIfConfig.md b/Sources/SwiftIfConfig/SwiftIfConfig.docc/SwiftIfConfig.md index e2a4636650e..77292721c0f 100644 --- a/Sources/SwiftIfConfig/SwiftIfConfig.docc/SwiftIfConfig.md +++ b/Sources/SwiftIfConfig/SwiftIfConfig.docc/SwiftIfConfig.md @@ -29,4 +29,5 @@ The `SwiftIfConfig` library provides utilities to determine which syntax nodes a * and are visitor types that only visit the syntax nodes that are included ("active") for a given build configuration, implicitly skipping any nodes within inactive `#if` clauses. * `SyntaxProtocol.removingInactive(in:)` produces a syntax node that removes all inactive regions (and their corresponding `IfConfigDeclSyntax` nodes) from the given syntax tree, returning a new tree that is free of `#if` conditions. * `IfConfigDeclSyntax.activeClause(in:)` determines which of the clauses of an `#if` is active for the given build configuration, returning the active clause. -* `SyntaxProtocol.configuredRegions(in:)` produces a `ConfiguredRegions` value that can be used to efficiently test whether a given syntax node is in an active, inactive, or unparsed region (via `isActive`). +* `SyntaxProtocol.configuredRegions(in:)` produces a `ConfiguredRegions` value that can be used to efficiently test whether a given syntax node is in an active, inactive, or unparsed region, remove inactive syntax, or determine the + active clause for a given `#if`. Use `ConfiguredRegions` for repeated queries. diff --git a/Tests/SwiftIfConfigTest/VisitorTests.swift b/Tests/SwiftIfConfigTest/VisitorTests.swift index b42a90e2412..0a4ea2fbd3c 100644 --- a/Tests/SwiftIfConfigTest/VisitorTests.swift +++ b/Tests/SwiftIfConfigTest/VisitorTests.swift @@ -23,26 +23,49 @@ import _SwiftSyntaxTestSupport /// /// This cross-checks the visitor itself with the `SyntaxProtocol.isActive(in:)` /// API. -class AllActiveVisitor: ActiveSyntaxAnyVisitor { - init(configuration: TestingBuildConfiguration) { - super.init(viewMode: .sourceAccurate, configuration: configuration) +class AllActiveVisitor: ActiveSyntaxAnyVisitor { + let configuration: TestingBuildConfiguration + + init( + configuration: TestingBuildConfiguration, + configuredRegions: ConfiguredRegions? = nil + ) { + self.configuration = configuration + + if let configuredRegions { + super.init(viewMode: .sourceAccurate, configuredRegions: configuredRegions) + } else { + super.init(viewMode: .sourceAccurate, configuration: configuration) + } } + open override func visitAny(_ node: Syntax) -> SyntaxVisitorContinueKind { XCTAssertEqual(node.isActive(in: configuration).state, .active) return .visitChildren } } -class NameCheckingVisitor: ActiveSyntaxAnyVisitor { +class NameCheckingVisitor: ActiveSyntaxAnyVisitor { + let configuration: TestingBuildConfiguration + /// The set of names we are expected to visit. Any syntax nodes with /// names that aren't here will be rejected, and each of the names listed /// here must occur exactly once. var expectedNames: Set - init(configuration: TestingBuildConfiguration, expectedNames: Set) { + init( + configuration: TestingBuildConfiguration, + expectedNames: Set, + configuredRegions: ConfiguredRegions? = nil + ) { + self.configuration = configuration self.expectedNames = expectedNames - super.init(viewMode: .sourceAccurate, configuration: configuration) + if let configuredRegions { + super.init(viewMode: .sourceAccurate, configuredRegions: configuredRegions) + } else { + super.init(viewMode: .sourceAccurate, configuration: configuration) + } } deinit { @@ -145,32 +168,31 @@ public class VisitorTests: XCTestCase { func testAnyVisitorVisitsOnlyActive() throws { // Make sure that all visited nodes are active nodes. - AllActiveVisitor(configuration: linuxBuildConfig).walk(inputSource) - AllActiveVisitor(configuration: iosBuildConfig).walk(inputSource) + assertVisitedAllActive(linuxBuildConfig) + assertVisitedAllActive(iosBuildConfig) } func testVisitsExpectedNodes() throws { // Check that the right set of names is visited. - NameCheckingVisitor( - configuration: linuxBuildConfig, + assertVisitedExpectedNames( + linuxBuildConfig, expectedNames: ["f", "h", "i", "S", "generationCount", "value", "withAvail"] - ).walk(inputSource) + ) - NameCheckingVisitor( - configuration: iosBuildConfig, + assertVisitedExpectedNames( + iosBuildConfig, expectedNames: ["g", "h", "i", "a", "S", "generationCount", "value", "error", "withAvail"] - ).walk(inputSource) + ) } func testVisitorWithErrors() throws { var configuration = linuxBuildConfig configuration.badAttributes.insert("available") - let visitor = NameCheckingVisitor( - configuration: configuration, - expectedNames: ["f", "h", "i", "S", "generationCount", "value", "notAvail"] + assertVisitedExpectedNames( + configuration, + expectedNames: ["f", "h", "i", "S", "generationCount", "value", "notAvail"], + diagnosticCount: 3 ) - visitor.walk(inputSource) - XCTAssertEqual(visitor.diagnostics.count, 3) } func testRemoveInactive() { @@ -337,6 +359,44 @@ public class VisitorTests: XCTestCase { } } +extension VisitorTests { + /// Ensure that all visited nodes are active nodes according to the given + /// build configuration. + fileprivate func assertVisitedAllActive(_ configuration: TestingBuildConfiguration) { + AllActiveVisitor(configuration: configuration).walk(inputSource) + + let configuredRegions = inputSource.configuredRegions(in: configuration) + AllActiveVisitor( + configuration: configuration, + configuredRegions: configuredRegions + ).walk(inputSource) + } + + /// Ensure that we visit nodes with the set of names we were expecting to + /// visit. + fileprivate func assertVisitedExpectedNames( + _ configuration: TestingBuildConfiguration, + expectedNames: Set, + diagnosticCount: Int = 0 + ) { + let firstVisitor = NameCheckingVisitor( + configuration: configuration, + expectedNames: expectedNames + ) + firstVisitor.walk(inputSource) + XCTAssertEqual(firstVisitor.diagnostics.count, diagnosticCount) + + let configuredRegions = inputSource.configuredRegions(in: configuration) + let secondVisitor = NameCheckingVisitor( + configuration: configuration, + expectedNames: expectedNames, + configuredRegions: configuredRegions + ) + secondVisitor.walk(inputSource) + XCTAssertEqual(secondVisitor.diagnostics.count, diagnosticCount) + } +} + /// Assert that removing any inactive code according to the given build /// configuration returns the expected source and diagnostics. fileprivate func assertRemoveInactive(