Skip to content

Fix and improve parallel mode #261

New issue

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

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

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Sep 2, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion Package.swift
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,11 @@ let package = Package(
),
.target(
name: "SwiftFormatTestSupport",
dependencies: ["SwiftFormatCore", "SwiftFormatConfiguration"]
dependencies: [
"SwiftFormatCore",
"SwiftFormatRules",
"SwiftFormatConfiguration",
]
),
.target(
name: "SwiftFormatWhitespaceLinter",
Expand Down
4 changes: 2 additions & 2 deletions Sources/SwiftFormat/LintPipeline.swift
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ extension LintPipeline {
func visitIfEnabled<Rule: SyntaxLintRule, Node: SyntaxProtocol>(
_ visitor: (Rule) -> (Node) -> SyntaxVisitorContinueKind, for node: Node
) {
guard context.isRuleEnabled(Rule.self.ruleName, node: Syntax(node)) else { return }
guard context.isRuleEnabled(Rule.self, node: Syntax(node)) else { return }
let rule = self.rule(Rule.self)
_ = visitor(rule)(node)
}
Expand All @@ -50,7 +50,7 @@ extension LintPipeline {
// more importantly because the `visit` methods return protocol refinements of `Syntax` that
// cannot currently be expressed as constraints without duplicating this function for each of
// them individually.
guard context.isRuleEnabled(Rule.self.ruleName, node: Syntax(node)) else { return }
guard context.isRuleEnabled(Rule.self, node: Syntax(node)) else { return }
let rule = self.rule(Rule.self)
_ = visitor(rule)(node)
}
Expand Down
3 changes: 2 additions & 1 deletion Sources/SwiftFormat/SwiftFormatter.swift
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import Foundation
import SwiftFormatConfiguration
import SwiftFormatCore
import SwiftFormatPrettyPrint
import SwiftFormatRules
import SwiftSyntax

/// Formats Swift source code or syntax trees according to the Swift style guidelines.
Expand Down Expand Up @@ -108,7 +109,7 @@ public final class SwiftFormatter {
let assumedURL = url ?? URL(fileURLWithPath: "source")
let context = Context(
configuration: configuration, diagnosticEngine: diagnosticEngine, fileURL: assumedURL,
sourceFileSyntax: syntax, source: source)
sourceFileSyntax: syntax, source: source, ruleNameCache: ruleNameCache)
let pipeline = FormatPipeline(context: context)
let transformedSyntax = pipeline.visit(Syntax(syntax))

Expand Down
3 changes: 2 additions & 1 deletion Sources/SwiftFormat/SwiftLinter.swift
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import Foundation
import SwiftFormatConfiguration
import SwiftFormatCore
import SwiftFormatPrettyPrint
import SwiftFormatRules
import SwiftFormatWhitespaceLinter
import SwiftSyntax

Expand Down Expand Up @@ -88,7 +89,7 @@ public final class SwiftLinter {

let context = Context(
configuration: configuration, diagnosticEngine: diagnosticEngine, fileURL: url,
sourceFileSyntax: syntax, source: source)
sourceFileSyntax: syntax, source: source, ruleNameCache: ruleNameCache)
let pipeline = LintPipeline(context: context)
pipeline.walk(Syntax(syntax))

Expand Down
18 changes: 16 additions & 2 deletions Sources/SwiftFormatCore/Context.swift
Original file line number Diff line number Diff line change
Expand Up @@ -52,13 +52,17 @@ public class Context {
/// Contains the rules have been disabled by comments for certain line numbers.
public let ruleMask: RuleMask

/// Contains all the available rules' names associated to their types' object identifiers.
public let ruleNameCache: [ObjectIdentifier: String]

/// Creates a new Context with the provided configuration, diagnostic engine, and file URL.
public init(
configuration: Configuration,
diagnosticEngine: DiagnosticEngine?,
fileURL: URL,
sourceFileSyntax: SourceFileSyntax,
source: String? = nil
source: String? = nil,
ruleNameCache: [ObjectIdentifier: String]
) {
self.configuration = configuration
self.diagnosticEngine = diagnosticEngine
Expand All @@ -71,12 +75,22 @@ public class Context {
syntaxNode: Syntax(sourceFileSyntax),
sourceLocationConverter: sourceLocationConverter
)
self.ruleNameCache = ruleNameCache
}

/// Given a rule's name and the node it is examining, determine if the rule is disabled at this
/// location or not.
public func isRuleEnabled(_ ruleName: String, node: Syntax) -> Bool {
public func isRuleEnabled<R: Rule>(_ rule: R.Type, node: Syntax) -> Bool {
let loc = node.startLocation(converter: self.sourceLocationConverter)

assert(
ruleNameCache[ObjectIdentifier(rule)] != nil,
"""
Missing cached rule name for '\(rule)'! \
Ensure `generate-pipelines` has been run and `ruleNameCache` was injected.
""")

let ruleName = ruleNameCache[ObjectIdentifier(rule)] ?? R.ruleName
switch ruleMask.ruleState(ruleName, at: loc) {
case .default:
return configuration.rules[ruleName] ?? false
Expand Down
24 changes: 2 additions & 22 deletions Sources/SwiftFormatCore/Rule.swift
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ public protocol Rule {
/// The context in which the rule is executed.
var context: Context { get }

/// The human-readable name of the rule. This defaults to the class name.
/// The human-readable name of the rule. This defaults to the type name.
static var ruleName: String { get }

/// Whether this rule is opt-in, meaning it is disabled by default.
Expand All @@ -27,27 +27,7 @@ public protocol Rule {
init(context: Context)
}

fileprivate var nameCache = [ObjectIdentifier: String]()
fileprivate var nameCacheQueue = DispatchQueue(
label: "com.apple.SwiftFormat.NameCache", attributes: .concurrent)

extension Rule {
/// By default, the `ruleName` is just the name of the implementing rule class.
public static var ruleName: String {
let identifier = ObjectIdentifier(self)
let cachedName = nameCacheQueue.sync {
nameCache[identifier]
}

if let cachedName = cachedName {
return cachedName
}

let name = String("\(self)".split(separator: ".").last!)
nameCacheQueue.async(flags: .barrier) {
nameCache[identifier] = name
}

return name
}
public static var ruleName: String { String("\(self)".split(separator: ".").last!) }
}
2 changes: 1 addition & 1 deletion Sources/SwiftFormatCore/SyntaxFormatRule.swift
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ open class SyntaxFormatRule: SyntaxRewriter, Rule {
open override func visitAny(_ node: Syntax) -> Syntax? {
// If the rule is not enabled, then return the node unmodified; otherwise, returning nil tells
// SwiftSyntax to continue with the standard dispatch.
guard context.isRuleEnabled(Self.ruleName, node: node) else { return node }
guard context.isRuleEnabled(type(of: self), node: node) else { return node }
return nil
}
}
2 changes: 1 addition & 1 deletion Sources/SwiftFormatRules/OrderedImports.swift
Original file line number Diff line number Diff line change
Expand Up @@ -324,7 +324,7 @@ fileprivate func generateLines(codeBlockItemList: CodeBlockItemListSyntax, conte
lines.append(currentLine)
currentLine = Line()
}
let sortable = context.isRuleEnabled(OrderedImports.ruleName, node: Syntax(block))
let sortable = context.isRuleEnabled(OrderedImports.self, node: Syntax(block))
currentLine.syntaxNode = .importCodeBlock(block, sortable: sortable)
} else {
guard let syntaxNode = currentLine.syntaxNode else {
Expand Down
51 changes: 51 additions & 0 deletions Sources/SwiftFormatRules/RuleNameCache+Generated.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
//===----------------------------------------------------------------------===//
//
// This source file is part of the Swift.org open source project
//
// Copyright (c) 2014 - 2019 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
//
//===----------------------------------------------------------------------===//

// This file is automatically generated with generate-pipeline. Do Not Edit!

/// By default, the `Rule.ruleName` should be the name of the implementing rule type.
public let ruleNameCache: [ObjectIdentifier: String] = [
ObjectIdentifier(AllPublicDeclarationsHaveDocumentation.self): "AllPublicDeclarationsHaveDocumentation",
ObjectIdentifier(AlwaysUseLowerCamelCase.self): "AlwaysUseLowerCamelCase",
ObjectIdentifier(AmbiguousTrailingClosureOverload.self): "AmbiguousTrailingClosureOverload",
ObjectIdentifier(BeginDocumentationCommentWithOneLineSummary.self): "BeginDocumentationCommentWithOneLineSummary",
ObjectIdentifier(DoNotUseSemicolons.self): "DoNotUseSemicolons",
ObjectIdentifier(DontRepeatTypeInStaticProperties.self): "DontRepeatTypeInStaticProperties",
ObjectIdentifier(FileScopedDeclarationPrivacy.self): "FileScopedDeclarationPrivacy",
ObjectIdentifier(FullyIndirectEnum.self): "FullyIndirectEnum",
ObjectIdentifier(GroupNumericLiterals.self): "GroupNumericLiterals",
ObjectIdentifier(IdentifiersMustBeASCII.self): "IdentifiersMustBeASCII",
ObjectIdentifier(NeverForceUnwrap.self): "NeverForceUnwrap",
ObjectIdentifier(NeverUseForceTry.self): "NeverUseForceTry",
ObjectIdentifier(NeverUseImplicitlyUnwrappedOptionals.self): "NeverUseImplicitlyUnwrappedOptionals",
ObjectIdentifier(NoAccessLevelOnExtensionDeclaration.self): "NoAccessLevelOnExtensionDeclaration",
ObjectIdentifier(NoBlockComments.self): "NoBlockComments",
ObjectIdentifier(NoCasesWithOnlyFallthrough.self): "NoCasesWithOnlyFallthrough",
ObjectIdentifier(NoEmptyTrailingClosureParentheses.self): "NoEmptyTrailingClosureParentheses",
ObjectIdentifier(NoLabelsInCasePatterns.self): "NoLabelsInCasePatterns",
ObjectIdentifier(NoLeadingUnderscores.self): "NoLeadingUnderscores",
ObjectIdentifier(NoParensAroundConditions.self): "NoParensAroundConditions",
ObjectIdentifier(NoVoidReturnOnFunctionSignature.self): "NoVoidReturnOnFunctionSignature",
ObjectIdentifier(OneCasePerLine.self): "OneCasePerLine",
ObjectIdentifier(OneVariableDeclarationPerLine.self): "OneVariableDeclarationPerLine",
ObjectIdentifier(OnlyOneTrailingClosureArgument.self): "OnlyOneTrailingClosureArgument",
ObjectIdentifier(OrderedImports.self): "OrderedImports",
ObjectIdentifier(ReturnVoidInsteadOfEmptyTuple.self): "ReturnVoidInsteadOfEmptyTuple",
ObjectIdentifier(UseEarlyExits.self): "UseEarlyExits",
ObjectIdentifier(UseLetInEveryBoundCaseVariable.self): "UseLetInEveryBoundCaseVariable",
ObjectIdentifier(UseShorthandTypeNames.self): "UseShorthandTypeNames",
ObjectIdentifier(UseSingleLinePropertyGetter.self): "UseSingleLinePropertyGetter",
ObjectIdentifier(UseSynthesizedInitializer.self): "UseSynthesizedInitializer",
ObjectIdentifier(UseTripleSlashForDocumentationComments.self): "UseTripleSlashForDocumentationComments",
ObjectIdentifier(UseWhereClausesInForLoops.self): "UseWhereClausesInForLoops",
ObjectIdentifier(ValidateDocumentationComments.self): "ValidateDocumentationComments",
]
4 changes: 3 additions & 1 deletion Sources/SwiftFormatTestSupport/DiagnosingTestCase.swift
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import SwiftFormatConfiguration
import SwiftFormatCore
import SwiftFormatRules
import SwiftSyntax
import XCTest

Expand Down Expand Up @@ -39,7 +40,8 @@ open class DiagnosingTestCase: XCTestCase {
configuration: configuration ?? Configuration(),
diagnosticEngine: DiagnosticEngine(),
fileURL: URL(fileURLWithPath: "/tmp/test.swift"),
sourceFileSyntax: sourceFileSyntax)
sourceFileSyntax: sourceFileSyntax,
ruleNameCache: ruleNameCache)
consumer = DiagnosticTrackingConsumer()
context.diagnosticEngine?.addConsumer(consumer)
return context
Expand Down
55 changes: 55 additions & 0 deletions Sources/generate-pipeline/RuleNameCacheGenerator.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
//===----------------------------------------------------------------------===//
//
// This source file is part of the Swift.org open source project
//
// Copyright (c) 2014 - 2019 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 Foundation

/// Generates the rule registry file used to populate the default configuration.
final class RuleNameCacheGenerator: FileGenerator {

/// The rules collected by scanning the formatter source code.
let ruleCollector: RuleCollector

/// Creates a new rule registry generator.
init(ruleCollector: RuleCollector) {
self.ruleCollector = ruleCollector
}

func write(into handle: FileHandle) throws {
handle.write(
"""
//===----------------------------------------------------------------------===//
//
// This source file is part of the Swift.org open source project
//
// Copyright (c) 2014 - 2019 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
//
//===----------------------------------------------------------------------===//
// This file is automatically generated with generate-pipeline. Do Not Edit!
/// By default, the `Rule.ruleName` should be the name of the implementing rule type.
public let ruleNameCache: [ObjectIdentifier: String] = [
"""
)

for detectedRule in ruleCollector.allLinters.sorted(by: { $0.typeName < $1.typeName }) {
handle.write(" ObjectIdentifier(\(detectedRule.typeName).self): \"\(detectedRule.typeName)\",\n")
}
handle.write("]\n")
}
}

8 changes: 8 additions & 0 deletions Sources/generate-pipeline/main.swift
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,10 @@ let ruleRegistryFile = sourcesDirectory
.appendingPathComponent("SwiftFormatConfiguration")
.appendingPathComponent("RuleRegistry+Generated.swift")

let ruleNameCacheFile = sourcesDirectory
.appendingPathComponent("SwiftFormatRules")
.appendingPathComponent("RuleNameCache+Generated.swift")

var ruleCollector = RuleCollector()
try ruleCollector.collect(from: rulesDirectory)

Expand All @@ -34,3 +38,7 @@ try pipelineGenerator.generateFile(at: pipelineFile)
// Generate the rule registry dictionary for configuration.
let registryGenerator = RuleRegistryGenerator(ruleCollector: ruleCollector)
try registryGenerator.generateFile(at: ruleRegistryFile)

// Generate the rule name cache.
let ruleNameCacheGenerator = RuleNameCacheGenerator(ruleCollector: ruleCollector)
try ruleNameCacheGenerator.generateFile(at: ruleNameCacheFile)
24 changes: 10 additions & 14 deletions Sources/swift-format/Frontend/Frontend.swift
Original file line number Diff line number Diff line change
Expand Up @@ -130,35 +130,31 @@ class Frontend {
"processPaths(_:) should only be called when paths is non-empty.")

if parallel {
let allFilePaths = Array(FileIterator(paths: paths))
DispatchQueue.concurrentPerform(iterations: allFilePaths.count) { index in
let path = allFilePaths[index]
openAndProcess(path)
let filesToProcess = FileIterator(paths: paths).compactMap(openAndPrepareFile)
DispatchQueue.concurrentPerform(iterations: filesToProcess.count) { index in
processFile(filesToProcess[index])
}
} else {
for path in FileIterator(paths: paths) {
openAndProcess(path)
}
FileIterator(paths: paths).lazy.compactMap(openAndPrepareFile).forEach(processFile)
}
}

/// Read and process the given path, optionally synchronizing diagnostic output.
private func openAndProcess(_ path: String) -> Void {
/// Read and prepare the file at the given path for processing, optionally synchronizing
/// diagnostic output.
private func openAndPrepareFile(atPath path: String) -> FileToProcess? {
guard let sourceFile = FileHandle(forReadingAtPath: path) else {
diagnosticEngine.diagnose(Diagnostic.Message(.error, "Unable to open \(path)"))
return
return nil
}

guard let configuration = configuration(
atPath: lintFormatOptions.configurationPath, orInferredFromSwiftFileAtPath: path)
else {
// Already diagnosed in the called method.
return
return nil
}

let fileToProcess = FileToProcess(
fileHandle: sourceFile, path: path, configuration: configuration)
processFile(fileToProcess)
return FileToProcess(fileHandle: sourceFile, path: path, configuration: configuration)
}

/// Returns the configuration that applies to the given `.swift` source file, when an explicit
Expand Down