Skip to content

Commit 1defcb0

Browse files
committed
Refactor DiagnosticsEngine
This greatly simplifies how diagnostics are created and emitted in SwiftPM. The old statergy was a little complex and it didn't pull off its weight. It was written before ExpressibleByStringInterpolation was redesigned so it didn't really make sense to continue with this model. There was also a big overhead for creating and emitting new diagnostics as each diagnostic required a bunch of boilerplate setup code. This PR redesigns the class with inspiration from SwiftSyntax's DiagnosticsEngine. Diagnostics are now cheap to create and easy to emit. In the general case, it isn't necessary to create a dedicated type for individual diagnostic but there is a facility to do when necessary. We can also attach an unique ID to individual untyped diagnostic by using Swift's #function but I'll avoid that until we really need it. There is still some cleanup work left to do but that will come in later PRs. <rdar://problem/49466682>
1 parent 2dd8c48 commit 1defcb0

31 files changed

+461
-1469
lines changed

Sources/Basic/DiagnosticsEngine.swift

Lines changed: 77 additions & 223 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
/*
22
This source file is part of the Swift.org open source project
33

4-
Copyright (c) 2014 - 2017 Apple Inc. and the Swift project authors
4+
Copyright (c) 2014 - 2019 Apple Inc. and the Swift project authors
55
Licensed under Apache License v2.0 with Runtime Library Exception
66

77
See http://swift.org/LICENSE.txt for license information
@@ -10,174 +10,18 @@
1010

1111
import Dispatch
1212

13-
/// A type which can be used as a diagnostic parameter.
14-
public protocol DiagnosticParameter {
15-
16-
/// Human readable diagnostic description of the parameter.
17-
var diagnosticDescription: String { get }
18-
}
19-
20-
// Default implementation for types which conform to CustomStringConvertible.
21-
extension DiagnosticParameter where Self: CustomStringConvertible {
22-
public var diagnosticDescription: String {
23-
return description
24-
}
25-
}
26-
27-
// Conform basic types.
28-
extension String: DiagnosticParameter {}
29-
extension Int: DiagnosticParameter {}
30-
31-
/// A builder for constructing diagnostic descriptions.
32-
public class DiagnosticDescriptionBuilder<Data: DiagnosticData> {
33-
public var fragments: [DiagnosticID.DescriptionFragment] = []
34-
35-
func build(
36-
_ body: (DiagnosticDescriptionBuilder) -> Void
37-
) -> [DiagnosticID.DescriptionFragment] {
38-
body(self)
39-
return fragments
40-
}
41-
}
42-
43-
@discardableResult
44-
public func <<< <T>(
45-
builder: DiagnosticDescriptionBuilder<T>,
46-
string: String
47-
) -> DiagnosticDescriptionBuilder<T> {
48-
builder.fragments.append(.literal(string, preference: .default))
49-
return builder
50-
}
51-
52-
@discardableResult
53-
public func <<<<T, P: DiagnosticParameter>(
54-
builder: DiagnosticDescriptionBuilder<T>,
55-
accessor: @escaping (T) -> P
56-
) -> DiagnosticDescriptionBuilder<T> {
57-
builder.fragments.append(.substitution({ accessor($0 as! T) }, preference: .default))
58-
return builder
59-
}
60-
61-
@discardableResult
62-
public func <<< <T>(
63-
builder: DiagnosticDescriptionBuilder<T>,
64-
fragment: DiagnosticID.DescriptionFragment
65-
) -> DiagnosticDescriptionBuilder<T> {
66-
builder.fragments.append(fragment)
67-
return builder
68-
}
69-
70-
// FIXME: One thing we should consider is whether we should make the diagnostic
71-
// a protocol and put these properties on the type, which is conceptually the
72-
// right modeling, but might be cumbersome of our desired features don't
73-
// perfectly align with what the language can express.
74-
//
75-
/// A unique identifier for a diagnostic.
76-
///
77-
/// Diagnostic identifiers are intended to be a stable representation of a
78-
/// particular kind of diagnostic that can be emitted by the client.
79-
///
80-
/// The stabilty of identifiers is important for use cases where the client
81-
/// (e.g., a command line tool, an IDE, or a web UI) is expecting to receive
82-
/// diagnostics and be able to present additional UI affordances or workflows
83-
/// associated for specific kinds of diagnostics.
84-
public class DiagnosticID: ObjectIdentifierProtocol {
85-
86-
/// A piece of a diagnostic description.
87-
public enum DescriptionFragment {
88-
/// Represents how important a fragment is.
89-
public enum Preference {
90-
case low, `default`, high
91-
}
92-
93-
/// A literal string.
94-
case literalItem(String, preference: Preference)
95-
96-
/// A substitution of a computed value.
97-
case substitutionItem((DiagnosticData) -> DiagnosticParameter, preference: Preference)
98-
99-
public static func literal(
100-
_ string: String,
101-
preference: Preference = .default
102-
) -> DescriptionFragment {
103-
return .literalItem(string, preference: preference)
104-
}
105-
106-
public static func substitution(
107-
_ accessor: @escaping ((DiagnosticData) -> DiagnosticParameter),
108-
preference: Preference = .default
109-
) -> DescriptionFragment {
110-
return .substitutionItem(accessor, preference: preference)
111-
}
112-
}
113-
114-
/// The name of the diagnostic, which is expected to be in reverse dotted notation.
115-
public let name: String
116-
117-
/// The English format string for the diagnostic description.
118-
public let description: [DescriptionFragment]
119-
120-
/// The default behavior associated with this diagnostic.
121-
public let defaultBehavior: Diagnostic.Behavior
122-
123-
/// Create a new diagnostic identifier.
124-
///
125-
/// - Parameters:
126-
/// - type: The type of the payload data, used to help type inference.
127-
///
128-
/// - name: The name of the identifier.
129-
///
130-
/// - description: A closure which will compute the description from a
131-
/// builder. We compute descriptions in this fashion in
132-
/// order to take advantage of type inference to make it
133-
/// easy to have type safe accessors for the payload
134-
/// properties.
135-
///
136-
/// The intended use is to support a convenient inline syntax for defining
137-
/// new diagnostics, for example:
138-
///
139-
/// struct TooFewLives: DiagnosticData {
140-
/// static var id = DiagnosticID(
141-
/// type: TooFewLives.self,
142-
/// name: "org.swift.diags.too-few-lives",
143-
/// description: { $0 <<< "cannot create a cat with" <<< { $0.count } <<< "lives" }
144-
/// )
145-
///
146-
/// let count: Int
147-
/// }
148-
public init<T>(
149-
type: T.Type,
150-
name: String,
151-
defaultBehavior: Diagnostic.Behavior = .error,
152-
description buildDescription: (DiagnosticDescriptionBuilder<T>) -> Void
153-
) {
154-
self.name = name
155-
self.description = DiagnosticDescriptionBuilder<T>().build(buildDescription)
156-
self.defaultBehavior = defaultBehavior
157-
}
158-
}
159-
16013
/// The payload of a diagnostic.
16114
public protocol DiagnosticData: CustomStringConvertible {
162-
/// The identifier of the diagnostic this payload corresponds to.
163-
static var id: DiagnosticID { get }
16415
}
16516

16617
extension DiagnosticData {
167-
public var description: String {
168-
return localizedDescription(for: self)
169-
}
18+
public var localizedDescription: String { self.description }
17019
}
17120

172-
/// The location of the diagnostic.
173-
public protocol DiagnosticLocation {
174-
/// The human readable summary description for the location.
175-
var localizedDescription: String { get }
21+
public protocol DiagnosticLocation: CustomStringConvertible {
17622
}
17723

178-
public struct Diagnostic {
179-
public typealias Location = DiagnosticLocation
180-
24+
public struct Diagnostic: CustomStringConvertible {
18125
/// The behavior associated with this diagnostic.
18226
public enum Behavior {
18327
/// An error which will halt the operation.
@@ -186,71 +30,55 @@ public struct Diagnostic {
18630
/// A warning, but which will not halt the operation.
18731
case warning
18832

189-
/// An informational message.
19033
case note
19134

192-
/// A diagnostic which was ignored.
35+
// FIXME: Kill this
19336
case ignored
19437
}
19538

196-
/// The diagnostic identifier.
197-
public var id: DiagnosticID {
198-
return type(of: data).id
39+
public struct Message {
40+
/// The diagnostic's behavior.
41+
public let behavior: Behavior
42+
43+
/// The information on the actual diagnostic.
44+
public let data: DiagnosticData
45+
46+
/// The textual representation of the diagnostic data.
47+
public var text: String { data.description }
48+
49+
fileprivate init(data: DiagnosticData, behavior: Behavior) {
50+
self.data = data
51+
self.behavior = behavior
52+
}
19953
}
20054

201-
/// The diagnostic's behavior.
202-
public let behavior: Behavior
55+
/// The message in this diagnostic.
56+
public let message: Message
20357

20458
/// The conceptual location of this diagnostic.
20559
///
20660
/// This could refer to a concrete location in a file, for example, but it
20761
/// could also refer to an abstract location such as "the Git repository at
20862
/// this URL".
209-
public let location: Location
63+
public let location: DiagnosticLocation
21064

211-
/// The information on the actual diagnostic.
212-
public let data: DiagnosticData
65+
public var data: DiagnosticData { message.data }
66+
public var behavior: Behavior { message.behavior }
21367

214-
// FIXME: Need additional attachment mechanism (backtrace, etc.), or
215-
// extensible handlers (e.g., interactive diagnostics).
216-
217-
/// Create a new diagnostic.
218-
///
219-
/// - Parameters:
220-
/// - location: The abstract location of the issue which triggered the diagnostic.
221-
/// - parameters: The parameters to the diagnostic conveying additional information.
222-
/// - Precondition: The bindings must match those declared by the identifier.
223-
public init(location: Location, data: DiagnosticData) {
224-
// FIXME: Implement behavior overrides.
225-
self.behavior = type(of: data).id.defaultBehavior
68+
public init(
69+
message: Message,
70+
location: DiagnosticLocation = UnknownLocation.location
71+
) {
72+
self.message = message
22673
self.location = location
227-
self.data = data
22874
}
22975

230-
/// The human readable summary description for the diagnostic.
231-
public var localizedDescription: String {
232-
return Basic.localizedDescription(for: data)
233-
}
234-
}
76+
public var description: String { message.text }
23577

236-
/// A scope of diagnostics.
237-
///
238-
/// The scope provides aggregate information on all of the diagnostics which can
239-
/// be produced by a component.
240-
public protocol DiagnosticsScope {
241-
/// Get a URL for more information on a particular diagnostic ID.
242-
///
243-
/// This is intended to be used to provide verbose descriptions for diagnostics.
244-
///
245-
/// - Parameters:
246-
/// - id: The diagnostic ID to describe.
247-
/// - diagnostic: If provided, a specific diagnostic to describe.
248-
/// - Returns: If available, a URL which will give more information about
249-
/// the diagnostic.
250-
func url(describing id: DiagnosticID, for diagnostic: Diagnostic?) -> String?
78+
public var localizedDescription: String { message.text }
25179
}
25280

253-
public class DiagnosticsEngine: CustomStringConvertible {
81+
public final class DiagnosticsEngine: CustomStringConvertible {
25482

25583
public typealias DiagnosticsHandler = (Diagnostic) -> Void
25684

@@ -273,16 +101,21 @@ public class DiagnosticsEngine: CustomStringConvertible {
273101

274102
/// Returns true if there is an error diagnostics in the engine.
275103
public var hasErrors: Bool {
276-
return diagnostics.contains(where: { $0.behavior == .error })
104+
return diagnostics.contains(where: { $0.message.behavior == .error })
277105
}
278106

279107
public init(handlers: [DiagnosticsHandler] = []) {
280108
self.handlers = handlers
281109
}
282110

283-
public func emit(data: DiagnosticData, location: DiagnosticLocation) {
284-
let diagnostic = Diagnostic(location: location, data: data)
111+
public func emit(
112+
_ message: Diagnostic.Message,
113+
location: DiagnosticLocation = UnknownLocation.location
114+
) {
115+
emit(Diagnostic(message: message, location: location))
116+
}
285117

118+
private func emit(_ diagnostic: Diagnostic) {
286119
queue.sync {
287120
_diagnostics.append(diagnostic)
288121
}
@@ -303,35 +136,56 @@ public class DiagnosticsEngine: CustomStringConvertible {
303136
/// Merges contents of given engine.
304137
public func merge(_ engine: DiagnosticsEngine) {
305138
for diagnostic in engine.diagnostics {
306-
emit(data: diagnostic.data, location: diagnostic.location)
139+
emit(diagnostic.message, location: diagnostic.location)
307140
}
308141
}
309142

310143
public var description: String {
311144
let stream = BufferedOutputByteStream()
312145
stream <<< "["
313146
for diag in diagnostics {
314-
stream <<< diag.localizedDescription <<< ", "
147+
stream <<< diag.description <<< ", "
315148
}
316149
stream <<< "]"
317150
return stream.bytes.description
318151
}
319152
}
320153

321-
/// Returns localized description of a diagnostic data.
322-
fileprivate func localizedDescription(for data: DiagnosticData) -> String {
323-
var result = ""
324-
for (i, fragment) in type(of: data).id.description.enumerated() {
325-
if i != 0 {
326-
result += " "
327-
}
154+
extension Diagnostic.Message {
155+
public static func error(_ data: DiagnosticData) -> Diagnostic.Message {
156+
.init(data: data, behavior: .error)
157+
}
328158

329-
switch fragment {
330-
case let .literalItem(string, _):
331-
result += string
332-
case let .substitutionItem(accessor, _):
333-
result += accessor(data).diagnosticDescription
334-
}
159+
public static func warning(_ data: DiagnosticData) -> Diagnostic.Message {
160+
.init(data: data, behavior: .warning)
161+
}
162+
163+
public static func error(_ str: String) -> Diagnostic.Message {
164+
.error(StringDiagnostic(str))
165+
}
166+
167+
public static func warning(_ str: String) -> Diagnostic.Message {
168+
.warning(StringDiagnostic(str))
169+
}
170+
}
171+
172+
public struct StringDiagnostic: DiagnosticData {
173+
/// The diagnostic description.
174+
public let description: String
175+
176+
public init(_ description: String) {
177+
self.description = description
178+
}
179+
}
180+
181+
/// Represents a diagnostic location whic is unknown.
182+
public final class UnknownLocation: DiagnosticLocation {
183+
/// The singleton instance.
184+
public static let location = UnknownLocation()
185+
186+
private init() {}
187+
188+
public var description: String {
189+
return "<unknown>"
335190
}
336-
return result
337191
}

0 commit comments

Comments
 (0)