Add extraction for review discussions

- Add extraction logic
- Add Discussion struct and property to Proposal
- Add validation issues for missing and malformed discussions
- Update tests with new expected results
- Add proposals to Malformed snapshot to test missing or malformed field values

Author: dempseyatgithub

Sources/EvolutionMetadataExtraction/Extractors/FieldExtractors/DiscussionExtractor.swift

@@ -0,0 +1,66 @@
+
+// This source file is part of the Swift.org open source project
+//
+// Copyright (c) 2024 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 Markdown
+import EvolutionMetadataModel
+
+struct DiscussionExtractor: MarkupWalker, ValueExtractor {
+    
+    private var warnings: [Proposal.Issue] = []
+    private var errors: [Proposal.Issue] = []
+    
+    private var discussions: [Proposal.Discussion] = []
+
+    mutating func extractValue(from sourceValues: (headerFieldsByLabel: [String : ListItem], proposalID: String)) -> ExtractionResult<[Proposal.Discussion]> {
+        
+        // VALIDATION ENHANCEMENT: Normalize naming to 'Review' in the source proposals.
+        if let (_, headerField) = sourceValues.headerFieldsByLabel[["Review", "Reviews", "Decision Notes", "Decision notes"]] {
+            visit(headerField)
+            
+            // VALIDATION ENHANCEMENT: Correct proposals with known issues and remove special case logic.
+            // Currently a fair number of older proposals are missing links to discussions or do not
+            // format discussions correctly. Those issues should be corrected in the proposals themselves.
+            // Once all of those issues are resolved, the legacy check can be removed.
+            if discussions.isEmpty && !Legacy.discussionExtractionFailures.contains(sourceValues.proposalID) {
+                errors.append(ValidationIssue.discussionExtractionFailure)
+            }
+        } else {
+            // VALIDATION ENHANCEMENT: Add field to proposals with missing field and remove special case logic.
+            // 'Review' is a required field, but currently a fair number of older proposals are missing the
+            // field for a variety of reasons. Those issues should be corrected in the proposals themselves.
+            // Once all of those issues are resolved, the legacy check can be removed.
+            // Note that some very early proposals may not have valid discussions be extracted.
+            if !Legacy.missingReviewFields.contains(sourceValues.proposalID) {
+                errors.append(ValidationIssue.missingReviewField)
+            }
+        }
+        
+        return ExtractionResult(value: discussions, warnings: warnings, errors: errors)
+    }
+    
+    // Existing proposals with known validation errors.
+    // The listed exceptions will not generate warnings and errors for the known issues.
+    private enum Legacy {
+        static let missingReviewFields: Set<String> = ["SE-0001", "SE-0002", "SE-0004", "SE-0020", "SE-0051", "SE-0079", "SE-0100", "SE-0176", "SE-0177", "SE-0188", "SE-0193", "SE-0194", "SE-0196", "SE-0198", "SE-0201", "SE-0203", "SE-0205", "SE-0208", "SE-0209", "SE-0210", "SE-0212", "SE-0213", "SE-0219", "SE-0243", "SE-0245", "SE-0247", "SE-0248", "SE-0249", "SE-0250", "SE-0252", "SE-0259", "SE-0263", "SE-0268", "SE-0269", "SE-0273", "SE-0278", "SE-0284", "SE-0289", "SE-0295", "SE-0300", "SE-0303", "SE-0304", "SE-0312", "SE-0313", "SE-0317", "SE-0318", "SE-0337", "SE-0338", "SE-0341", "SE-0343", "SE-0344", "SE-0348", "SE-0350", "SE-0356", "SE-0365", "SE-0385"]
+        
+        static let discussionExtractionFailures: Set<String> = ["SE-0099", "SE-0363", "SE-0378", "SE-0391", "SE-0392"]
+    }
+    
+    mutating func visitLink(_ link: Link) -> () {
+        guard let linkInfo = LinkInfo(link: link) else {
+            return
+        }
+        
+        if let discussionURL = linkInfo.swiftForumsDestination {
+            discussions.append(Proposal.Discussion(name: linkInfo.text, link: discussionURL))
+        } else {
+            warnings.append(ValidationIssue.invalidDiscussionLink)
+        }
+    }
+}

Sources/EvolutionMetadataExtraction/Extractors/ProposalMetadataExtractor.swift

@@ -76,6 +76,12 @@ struct ProposalMetadataExtractor {
             proposal.trackingBugs = extractValue(from: headerFieldsByLabel, with: TrackingBugExtractor.self)
             proposal.implementation = extractValue(from: headerFieldsByLabel, with: ImplementationExtractor.self)
 
+            if let discussions = extractValue(from: (headerFieldsByLabel, proposalSpec.id), with: DiscussionExtractor.self) {
+                proposal.discussions = discussions
+            } else {
+                errors.append(ValidationIssue.missingReviewField)
+            }
+            
             if let status = extractValue(from: (headerFieldsByLabel, extractionDate), with: StatusExtractor.self) {
                 if case .implemented(let version) = status, version == "none" {
                     // VALIDATION ENHANCEMENT: Figure out a better way to special case the missing version strings for these proposals
@@ -167,6 +173,14 @@ struct LinkInfo {
             return nil
         }
     }
+    
+    var swiftForumsDestination: String? {
+        if destination.starts(with: "https://forums.swift.org") {
+            return destination
+        } else {
+            return nil
+        }
+    }
 }
 
 

Sources/EvolutionMetadataExtraction/Utilities/ValidationIssue.swift

@@ -76,6 +76,18 @@ enum ValidationIssue {
         message: "Failed to extract previous proposal IDs."
     )
 
+    static let missingReviewField = Proposal.Issue(
+        kind: .error,
+        stage: .parse,
+        message: "Missing Review field."
+    )
+
+    static let discussionExtractionFailure = Proposal.Issue(
+        kind: .error,
+        stage: .parse,
+        message: "Failed to extract discussions from Review field."
+    )
+
     // MARK: - Parse Warnings
 
     // VALIDATION ENHANCEMENT: Why is this only a warning?
@@ -166,6 +178,12 @@ enum ValidationIssue {
         stage: .validate,
         message: "Implementation links to a non-Swift repository."
     )
+    
+    static let invalidDiscussionLink = Proposal.Issue(
+        kind: .warning,
+        stage: .validate,
+        message: "Discussion link doesn't refer to a Swift forum thread. Discussion removed."
+    )
 
     // VALIDATION ENHANCEMENTS: Consider not including the date in the review message, or not including time components.
     // Comparing results currently breaks if recorded test values and actual values are generated in different time zones.

Sources/EvolutionMetadataModel/Proposal+Discussion.swift

@@ -0,0 +1,26 @@
+
+// This source file is part of the Swift.org open source project
+//
+// Copyright (c) 2024 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
+
+extension Proposal {
+    
+    /// Type that represents a discussion abou a proposal.
+    public struct Discussion: Sendable, Codable, Equatable {
+
+        /// Name of the discussion
+        public let name: String
+
+        /// URL string to the discussion
+        public let link: String
+        
+        public init(name: String = "", link: String = "") {
+            self.name = name
+            self.link = link
+        }
+    }
+}

Sources/EvolutionMetadataModel/Proposal.swift

@@ -114,6 +114,9 @@ public struct Proposal: Equatable, Sendable, Codable, Identifiable {
     /// Optional array of implementation links
     public var implementation: [Implementation]?
 
+    /// Array of discussions about the proposal. May be an empty array.
+    public var discussions: [Discussion]
+    
     /// Optional array of warnings
     ///
     /// Present only if validation warnings were found when extracting metadata from this proposal.
@@ -124,7 +127,7 @@ public struct Proposal: Equatable, Sendable, Codable, Identifiable {
     /// Present only if validation errors were found when extracting metadata from this proposal.
     public var errors: [Issue]?
 
-    public init(id: String = "", title: String = "", summary: String = "", link: String = "", sha: String = "", authors: [Person] = [], reviewManagers: [Person] = [], status: Status = .statusExtractionNotAttempted, trackingBugs: [TrackingBug]? = nil, implementation: [Implementation]? = nil, warnings: [Issue]? = nil, errors: [Issue]? = nil)
+    public init(id: String = "", title: String = "", summary: String = "", link: String = "", sha: String = "", authors: [Person] = [], reviewManagers: [Person] = [], status: Status = .statusExtractionNotAttempted, trackingBugs: [TrackingBug]? = nil, implementation: [Implementation]? = nil, discussions: [Discussion] = [], warnings: [Issue]? = nil, errors: [Issue]? = nil)
     {
         self.id = id
         self.title = title
@@ -137,6 +140,7 @@ public struct Proposal: Equatable, Sendable, Codable, Identifiable {
         self.status = status
         self.trackingBugs = trackingBugs
         self.implementation = implementation
+        self.discussions = discussions
         self.warnings = warnings
         self.errors = errors
     }