Skip to content

Commit f3887b0

Browse files
Add extraction for previous proposal IDs
- Add extraction logic - Add previousProposalIDs property to Proposal - Add validation issue for extraction failure - Update AllProposals test with new expected results - Add proposal to Malformed snapshot to test malformed field values
1 parent 44c5ad3 commit f3887b0

File tree

7 files changed

+209
-0
lines changed

7 files changed

+209
-0
lines changed
Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
2+
// This source file is part of the Swift.org open source project
3+
//
4+
// Copyright (c) 2024 Apple Inc. and the Swift project authors
5+
// Licensed under Apache License v2.0 with Runtime Library Exception
6+
//
7+
// See https://swift.org/LICENSE.txt for license information
8+
// See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors
9+
10+
import Markdown
11+
import EvolutionMetadataModel
12+
13+
struct PreviousProposalExtractor: MarkupWalker, ValueExtractor {
14+
15+
private var warnings: [Proposal.Issue] = []
16+
private var errors: [Proposal.Issue] = []
17+
18+
private var previousProposalIDs: [String]? {
19+
_previousProposalIDs.isEmpty ? nil : _previousProposalIDs
20+
}
21+
private var _previousProposalIDs: [String] = []
22+
23+
mutating func extractValue(from headerFieldsByLabel: [String : ListItem]) -> ExtractionResult<[String]> {
24+
25+
if let prevPropField = headerFieldsByLabel[["Previous Proposal", "Previous Proposals"]] {
26+
visit(prevPropField.value)
27+
28+
// validate that if the header field is here at least one proposal ID was found
29+
if _previousProposalIDs.isEmpty {
30+
errors.append(ValidationIssue.previousProposalIDsExtractionFailure)
31+
}
32+
}
33+
34+
return ExtractionResult(value: previousProposalIDs, warnings: warnings, errors: errors)
35+
}
36+
37+
mutating func visitText(_ text: Text) -> () {
38+
// VALIDATION ENHANCEMENT: Potentially look for missing links or text that does not include a proposal ID
39+
for match in text.string.matches(of: /SE-\d\d\d\d/) {
40+
_previousProposalIDs.append(String(match.0))
41+
}
42+
}
43+
}

Sources/EvolutionMetadataExtraction/Extractors/ProposalMetadataExtractor.swift

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,10 @@ struct ProposalMetadataExtractor {
6969

7070
proposal.upcomingFeatureFlag = extractValue(from: headerFieldsByLabel, with: UpcomingFeatureFlagExtractor.self)
7171

72+
// Known issue with SE-0255, should be resolved by https://github.com/apple/swift-evolution/pull/2411
73+
if proposalSpec.id != "SE-0255" {
74+
proposal.previousProposalIDs = extractValue(from: headerFieldsByLabel, with: PreviousProposalExtractor.self)
75+
}
7276
proposal.trackingBugs = extractValue(from: headerFieldsByLabel, with: TrackingBugExtractor.self)
7377
proposal.implementation = extractValue(from: headerFieldsByLabel, with: ImplementationExtractor.self)
7478

Sources/EvolutionMetadataExtraction/Utilities/ValidationIssue.swift

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,12 @@ enum ValidationIssue {
6969
stage: .parse,
7070
message: "Failed to extract upcoming feature flag."
7171
)
72+
73+
static let previousProposalIDsExtractionFailure = Proposal.Issue(
74+
kind: .error,
75+
stage: .parse,
76+
message: "Failed to extract previous proposal IDs."
77+
)
7278

7379
// MARK: - Parse Warnings
7480

Sources/EvolutionMetadataModel/Proposal.swift

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -105,6 +105,9 @@ public struct Proposal: Equatable, Sendable, Codable, Identifiable {
105105
/// Optional upcoming feature flag
106106
public var upcomingFeatureFlag: UpcomingFeatureFlag?
107107

108+
/// Optional array of previous proposal IDs in the format _SE-NNNN_
109+
public var previousProposalIDs: [String]?
110+
108111
/// Optional array of tracking bugs
109112
public var trackingBugs: [TrackingBug]?
110113

Tests/ExtractionTests/Resources/AllProposals.evosnapshot/expected-results.json

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3308,6 +3308,9 @@
33083308
],
33093309
"id" : "SE-0094",
33103310
"link" : "0094-sequence-function.md",
3311+
"previousProposalIDs" : [
3312+
"SE-0045"
3313+
],
33113314
"reviewManager" : {
33123315
"link" : "http:\/\/github.com\/lattner",
33133316
"name" : "Chris Lattner"
@@ -6508,6 +6511,9 @@
65086511
}
65096512
],
65106513
"link" : "0182-newline-escape-in-strings.md",
6514+
"previousProposalIDs" : [
6515+
"SE-0168"
6516+
],
65116517
"reviewManager" : {
65126518
"link" : "https:\/\/github.com\/lattner",
65136519
"name" : "Chris Lattner"
@@ -9000,6 +9006,9 @@
90009006
}
90019007
],
90029008
"link" : "0245-array-uninitialized-initializer.md",
9009+
"previousProposalIDs" : [
9010+
"SE-0223"
9011+
],
90039012
"reviewManager" : {
90049013
"link" : "https:\/\/github.com\/tkremenek",
90059014
"name" : "Ted Kremenek"
@@ -9433,6 +9442,9 @@
94339442
}
94349443
],
94359444
"link" : "0256-contiguous-collection.md",
9445+
"previousProposalIDs" : [
9446+
"SE-0237"
9447+
],
94369448
"reviewManager" : {
94379449
"link" : "https:\/\/github.com\/tkremenek",
94389450
"name" : "Ted Kremenek"
@@ -14981,6 +14993,9 @@
1498114993
],
1498214994
"id" : "SE-0398",
1498314995
"link" : "0398-variadic-types.md",
14996+
"previousProposalIDs" : [
14997+
"SE-0393"
14998+
],
1498414999
"reviewManager" : {
1498515000
"link" : "https:\/\/github.com\/Jumhyn",
1498615001
"name" : "Frederick Kellison-Linn"
@@ -15012,6 +15027,10 @@
1501215027
],
1501315028
"id" : "SE-0399",
1501415029
"link" : "0399-tuple-of-value-pack-expansion.md",
15030+
"previousProposalIDs" : [
15031+
"SE-0393",
15032+
"SE-0398"
15033+
],
1501515034
"reviewManager" : {
1501615035
"link" : "https:\/\/github.com\/xwu",
1501715036
"name" : "Xiaodi Wu"
@@ -15501,6 +15520,13 @@
1550115520
],
1550215521
"id" : "SE-0412",
1550315522
"link" : "0412-strict-concurrency-for-global-variables.md",
15523+
"previousProposalIDs" : [
15524+
"SE-0302",
15525+
"SE-0306",
15526+
"SE-0316",
15527+
"SE-0337",
15528+
"SE-0343"
15529+
],
1550415530
"reviewManager" : {
1550515531
"link" : "https:\/\/github.com\/hborla",
1550615532
"name" : "Holly Borla"
@@ -16071,6 +16097,9 @@
1607116097
],
1607216098
"id" : "SE-0427",
1607316099
"link" : "0427-noncopyable-generics.md",
16100+
"previousProposalIDs" : [
16101+
"SE-0390"
16102+
],
1607416103
"reviewManager" : {
1607516104
"link" : "https:\/\/github.com\/hborla",
1607616105
"name" : "Holly Borla"
@@ -16206,6 +16235,9 @@
1620616235
],
1620716236
"id" : "SE-0430",
1620816237
"link" : "0430-transferring-parameters-and-results.md",
16238+
"previousProposalIDs" : [
16239+
"SE-0414"
16240+
],
1620916241
"reviewManager" : {
1621016242
"link" : "https:\/\/github.com\/beccadax",
1621116243
"name" : "Becca Royal-Gordon"

Tests/ExtractionTests/Resources/Malformed.evosnapshot/expected-results.json

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1094,6 +1094,54 @@
10941094
},
10951095
"summary" : "Going back and forth from Strings to their byte representations is an important part of solving many problems, including object serialization, binary and text file formats, wire\/network interfaces, and cryptography. Swift has such utilities, but currently only exposed through `String.Type.fromCString(_:)` and `String.Type.fromCStringRepairingIllFormedUTF8(_:)`.",
10961096
"title" : "Expose code unit initializers on String"
1097+
},
1098+
{
1099+
"authors" : [
1100+
{
1101+
"link" : "http:\/\/github.com\/erica",
1102+
"name" : "Erica Sadun"
1103+
}
1104+
],
1105+
"errors" : [
1106+
{
1107+
"code" : 0,
1108+
"kind" : "error",
1109+
"message" : "Failed to extract previous proposal IDs.",
1110+
"stage" : "parse",
1111+
"suggestion" : ""
1112+
}
1113+
],
1114+
"id" : "SE-0028",
1115+
"link" : "0028-missing-previous-proposal-id.md",
1116+
"reviewManager" : {
1117+
"link" : "https:\/\/github.com\/lattner",
1118+
"name" : "Chris Lattner"
1119+
},
1120+
"reviewManagers" : [
1121+
{
1122+
"link" : "https:\/\/github.com\/lattner",
1123+
"name" : "Chris Lattner"
1124+
}
1125+
],
1126+
"sha" : "",
1127+
"status" : {
1128+
"state" : "implemented",
1129+
"version" : "2.2"
1130+
},
1131+
"summary" : "This proposal aims to eliminate Swift's use of \"[screaming snake case](https:\/\/en.wikipedia.org\/wiki\/Snake_case)\" like `__FILE__` and `__FUNCTION__` and replacing identifier instances with common [octothorpe-prefixed](https:\/\/en.wiktionary.org\/wiki\/octothorpe) lowercase `#identifier` representations.",
1132+
"title" : "Modernizing Swift's Debugging Identifiers",
1133+
"trackingBugs" : [
1134+
{
1135+
"assignee" : "",
1136+
"id" : "SR-669",
1137+
"link" : "https:\/\/bugs.swift.org\/browse\/SR-669",
1138+
"radar" : "",
1139+
"resolution" : "",
1140+
"status" : "",
1141+
"title" : "",
1142+
"updated" : ""
1143+
}
1144+
]
10971145
}
10981146
],
10991147
"schemaVersion" : "0.1.0",
Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,73 @@
1+
# Modernizing Swift's Debugging Identifiers
2+
3+
* Proposal: [SE-0028](0028-missing-previous-proposal-id.md)
4+
* Author: [Erica Sadun](http://github.com/erica)
5+
* Review Manager: [Chris Lattner](https://github.com/lattner)
6+
* Status: **Implemented (Swift 2.2)**
7+
* Decision Notes: [Rationale](https://forums.swift.org/t/accepted-se-0028-modernizing-swifts-debugging-identifiers-line-etc/1303)
8+
* Bug: [SR-669](https://bugs.swift.org/browse/SR-669)
9+
* Previous Proposal: [Swift API Guidelines](0023-api-guidelines.md)
10+
11+
## Introduction
12+
13+
This proposal aims to eliminate Swift's use of "[screaming snake case](https://en.wikipedia.org/wiki/Snake_case)" like `__FILE__` and `__FUNCTION__` and replacing identifier instances with common [octothorpe-prefixed](https://en.wiktionary.org/wiki/octothorpe) lowercase `#identifier` representations.
14+
15+
*The Swift-Evolution discussion of this topic took place in the "[Review] SE-0022: Referencing the Objective-C selector of a method" thread and then in its own "[\[Proposal\] Eliminating Swift's Screaming Snake Case Identifiers](https://forums.swift.org/t/proposal-eliminating-swifts-screaming-snake-case-identifiers/1165)" thread*
16+
17+
## Motivation
18+
19+
Swift offers built-in `__FILE__`, `__LINE__`, `__COLUMN__`, `__FUNCTION__`, and `__DSO_HANDLE__` identifiers. The first four expand to string and integer literals corresponding to a current location in source code. The last provides an `UnsafePointer` to the current dynamic shared object (.dylib or .so file). These features provide high utility for logging, both tracing execution and enabling developers to [capture error context](http://ericasadun.com/2015/08/27/capturing-context-swiftlang/).
20+
21+
The current identifiers owe their syntax to C's `__FILE__` and `__LINE__` macros. These are built into C's preprocessor and expanded before running the C-language parser. Swift's implementation differs from C's but offers similar functionality and, unfortunately, similar symbols. This proposal aims to break free of the historic chains of their unsightly screaming snake case, which look like boa constrictors [trying to digest](https://s-media-cache-ak0.pinimg.com/originals/59/ea/ee/59eaee788c31463b70e6e3d4fca5508f.jpg) fully swallowed keywords.
22+
23+
## Proposed solution
24+
25+
Using octothorpe-prefixed keywords offers several advantages:
26+
27+
* They match the existing `#available` keyword (D. Gregor)
28+
* They match SE-0022's already-accepted `#selector(...)` approach that reference a method's Objective-C selector (D. Gregor)
29+
* They support targeted code completion (D. Gregor)
30+
* They add a compiler-supported expression type that doesn't steal keywords, introducing a convention where `#` means "invoke compiler substitution logic here" (J. Rose)
31+
* They'd provide short-term solutions for a yet-as-undesigned macro system (D. Gregor)
32+
33+
## Detailed design
34+
35+
This proposal renames the following identifiers:
36+
37+
* `__FILE__` -> `#file`
38+
* `__LINE__` -> `#line`
39+
* `__COLUMN__` -> `#column`
40+
* `__FUNCTION__` -> `#function` (*Added during review*)
41+
* `__DSO_HANDLE__` -> `#dsohandle`
42+
43+
These identifiers retain the magic behavior of the existing `__LINE__` features: in a normal expression context, they expand to the location at that point. In a default argument context, they expand to the location of the caller.
44+
45+
Additional points to be considered by the Swift team for inclusion:
46+
47+
* Adding `#filename` to avoid using `lastPathComponent` on `#file` references.
48+
* Retaining `__FUNCTION__` to be renamed as `#function`. (*Accepted during review*)
49+
* Adopting a lower-case naming standard including `#dsohandle` and a potential future `#sourcelocation`.
50+
* Introducing `#symbol`, (e.g. Swift.Dictionary.Init(x:Int,y:String)), which summarizes context including module, type, and function. A fully qualified symbol enables users to access exactly the information they desire. It should contain parameter type information to properly identify member overloads.
51+
52+
53+
## Possible Future Extensions
54+
55+
[SR-198](https://bugs.swift.org/browse/SR-198) requested the coalescing of existing identifiers. A structured `#sourcelocation` identifier could be added as a follow-on if and when the Swift team decides to tackle a standardized source location type, which would provide individual field or keyword access.
56+
57+
In support of summaries, Remy Demerest writes, "[I] love the idea that source location would be one object that you can print to get the full story while still retaining the possibility to use each individual components as needed, which is probably the rarer case. I never find myself wanting only some of properties and usually don't include them simply because it takes longer to write the format properly, if I can get them all in one go it's certainly a win."
58+
59+
Should such a type be adopted, I'd recommend support for common output summary representations suitably differentiated for debug and release logging. Alternatively `#context`, `#releasecontext`, and `#debugcontext` summaries could be added independently of the adoption of `#sourcelocation`.
60+
61+
## Implementation notes
62+
63+
The octothorpe-delineated `#line` identifier already exists in Swift for resetting line numbers. Constraining the current `#line` directive to be the first token after a newline would disambiguate use. Alternatively, the context `#line` identifier could be renamed `#linenumber`.
64+
65+
## Review Acceptance and Modifications
66+
67+
The review of SE-0028 "Modernizing Swift's Debugging Identifiers" ran from January 29… February 2, 2016. The proposal has been *accepted*, with modifications:
68+
69+
* The core team agrees that we should rename all of the existing `__FILE__`, `__LINE__`, `__COLUMN__`, `__FUNCTION__`, and `__DSO_HANDLE__` symbols to lowercase equivalents in the `#` namespace: `#file`, `#line`, `#column`, `#function`, `#dsohandle`. This includes keeping `__FUNCTION__`, and making `#line` have the dual behavior of being a directive when it is the first token on a line, but an expression otherwise. Renaming these symbols improves uniformity within the Swift language, and keeping all of them provides a smooth transition path from the old syntax to the new syntax.
70+
71+
* The core team isn’t thrilled with the magic “first token on a line” whitespace behavior that `#line` will be getting, and would like to start a discussion about renaming the old `#line` directive to something more specific and tailored to its purpose. Once that name and syntax is settled, we can rename the directive and remove the whitespace rule.
72+
73+
* The core team requests that `#symbol` be split out into a separate proposal, because it needs more detailed design work, and is an additive feature. For example, it might be appealing to provide a `#mangledname` expression that provides the current symbol as a mangled name: when fed into a demangler, a more structured form of the current symbol would be available.

0 commit comments

Comments
 (0)