diff --git a/CHANGELOG.md b/CHANGELOG.md index a2c766e..e1ab0fc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,7 +8,8 @@ Add new items at the end of the relevant section under **Unreleased**. ### Additions -- Add `validate` subcommand to generate validate reports +- Add `validate` subcommand to generate validate reports ([#79]) +- Add `--pull-request` option to validate proposals in a pull request ([#86]) ### Changes @@ -82,3 +83,5 @@ This changelog's format is based on [Keep a Changelog](https://keepachangelog.co [#72]: https://github.com/swiftlang/swift-evolution-metadata-extractor/pull/72 [#74]: https://github.com/swiftlang/swift-evolution-metadata-extractor/pull/74 [#79]: https://github.com/swiftlang/swift-evolution-metadata-extractor/pull/79 +[#83]: https://github.com/swiftlang/swift-evolution-metadata-extractor/pull/83 +[#86]: https://github.com/swiftlang/swift-evolution-metadata-extractor/pull/86 diff --git a/README.md b/README.md index 38c01ea..edcdd71 100644 --- a/README.md +++ b/README.md @@ -101,12 +101,20 @@ The package vends the `EvolutionMetadataModel` library. The library defines `Cod ## Validation Reports The `validate` command will extract metadata and generate a validation report containing any validation errors or warnings found. By default the report will be written to standard out. -If errors are found the tool will exit with an error code. +To validate the proposal files in a pull request, use the `--pull-request` option with the pull request number. + + `swift-evolution-metadata-extractor validate --pull-request 2474` + + > Will write a validation report of the proposal files in pull request #2474. + +If validation errors are found the tool will exit with an error code. The validate command is intended to be run on pull requests to ensure malformed proposal files are not committed. +Note that the `--pull-request` option has a limit of 100 changed files. + ### Options -The validate subcommand has options that work similar to extract command options: +The validate subcommand also has options that work similar to extract command options: - Use the `--output-path` option (`-o`) to specify a different output location or filename. - Use the `--verbose` option (`-v`) for verbose output as the tool runs. @@ -117,7 +125,7 @@ The validate subcommand has options that work similar to extract command options Use the `` argument to validate one or more local proposal files. -The `--snapshot-path` and `` argument are mutually exclusive. +The `--pull-request`, `--snapshot-path` and `` argument are mutually exclusive. ## Snapshots for Development and Testing Use the `snapshot` subcommand to record snapshots. diff --git a/Sources/EvolutionMetadataExtraction/CommandLineSupport.swift b/Sources/EvolutionMetadataExtraction/CommandLineSupport.swift index 31c9965..435e573 100644 --- a/Sources/EvolutionMetadataExtraction/CommandLineSupport.swift +++ b/Sources/EvolutionMetadataExtraction/CommandLineSupport.swift @@ -45,13 +45,17 @@ public enum ArgumentValidation { _ = URLSession.customized // Reads and validates HTTP Proxy environment variables if present } - @Sendable public static func extractionSource(snapshotURL: URL?, proposalURLs: [URL]) throws -> ExtractionJob.Source { - if snapshotURL != nil && !proposalURLs.isEmpty { - throw ValidationError("Cannot provide both a --snapshot-path and arguments") + @Sendable public static func extractionSource(snapshotURL: URL?, proposalURLs: [URL], pullRequest: Int? = nil) throws -> ExtractionJob.Source { + if (snapshotURL != nil && !proposalURLs.isEmpty) || + (snapshotURL != nil && pullRequest != nil) || + (pullRequest != nil && !proposalURLs.isEmpty) { + throw ValidationError("--pull-request, --snapshot-path, and arguments are mutually exclusive") } else if let snapshotURL { return .snapshot(snapshotURL) } else if !proposalURLs.isEmpty { return .files(proposalURLs) + } else if let pullRequest { + return .pullRequest(pullRequest) } else { return .network } diff --git a/Sources/EvolutionMetadataExtraction/ExtractionJob.swift b/Sources/EvolutionMetadataExtraction/ExtractionJob.swift index a2cf3b0..4fd49d1 100644 --- a/Sources/EvolutionMetadataExtraction/ExtractionJob.swift +++ b/Sources/EvolutionMetadataExtraction/ExtractionJob.swift @@ -21,6 +21,7 @@ public struct ExtractionJob: Sendable { case network case snapshot(URL) case files([URL]) + case pullRequest(Int) } public enum Output: Sendable, Codable, Equatable { @@ -76,6 +77,8 @@ public struct ExtractionJob: Sendable { try await makeSnapshotExtractionJob(snapshotURL: snapshotURL, output: output, ignorePreviousResults: ignorePreviousResults, forcedExtractionIDs: forcedExtractionIDs, extractionDate: extractionDate) case .files(let fileURLs): try makeFilesExtractionJob(fileURLs: fileURLs, output: output, ignorePreviousResults: ignorePreviousResults, forcedExtractionIDs: forcedExtractionIDs, extractionDate: extractionDate) + case .pullRequest(let pullRequestID): + try await makePullRequestExtractionJob(pullRequestID: pullRequestID, output: output, ignorePreviousResults: ignorePreviousResults, forcedExtractionIDs: forcedExtractionIDs, extractionDate: extractionDate) } } } @@ -148,6 +151,31 @@ extension ExtractionJob { return ExtractionJob(output: output, snapshot: snapshot, proposalSpecs: proposalSpecs, previousResults: nil, forcedExtractionIDs: forcedExtractionIDs, jobMetadata: jobMetadata) } + private static func makePullRequestExtractionJob(pullRequestID: Int, output: Output, ignorePreviousResults: Bool, forcedExtractionIDs: [String], extractionDate: Date) async throws -> ExtractionJob { + + // Argument validation should ensure correct values. Assert to catch problems in usage in tests. + assert(ignorePreviousResults == true && forcedExtractionIDs.isEmpty, "Extraction from a pull request always ignores previous results and performs a full extraction") + + let proposalContentItems = try await GitHubFetcher.fetchPullRequestProposalList(for: pullRequestID) + + // The proposals/ directory may have subdirectories for proposals from specific workgroups. + // Proposals in those subdirectories are filtered out of this proposal specs array. + let proposalSpecs = proposalContentItems.enumerated().compactMap { + $1.proposalSpec(sortIndex: $0) + } + + let jobMetadata = JobMetadata(commit: "", extractionDate: extractionDate) + + let snapshot: Snapshot? + if case let .snapshot(destURL) = output { + snapshot = Snapshot(sourceURL: nil, destURL: destURL, proposalListing: nil, directoryContents: [], proposalSpecs: [], previousResults: nil, expectedResults: nil, branchInfo: nil, snapshotDate: extractionDate) + } else { + snapshot = nil + } + + return ExtractionJob(output: output, snapshot: snapshot, proposalSpecs: proposalSpecs, previousResults: nil, forcedExtractionIDs: forcedExtractionIDs, jobMetadata: jobMetadata) + } + static func previousResults(from url: URL, ignorePreviousResults: Bool) async throws -> EvolutionMetadata? { if ignorePreviousResults { return nil } diff --git a/Sources/EvolutionMetadataExtraction/Utilities/Networking.swift b/Sources/EvolutionMetadataExtraction/Utilities/Networking.swift index 9061afd..0aecce7 100644 --- a/Sources/EvolutionMetadataExtraction/Utilities/Networking.swift +++ b/Sources/EvolutionMetadataExtraction/Utilities/Networking.swift @@ -77,6 +77,12 @@ struct GitHubPullFileItem: Codable { var contents_url: String var patch: String + var isProposalFile: Bool { + status != "removed" && + filename.hasPrefix("proposals/") && + filename.hasSuffix(".md") + } + func proposalSpec(sortIndex: Int) -> ProposalSpec { ProposalSpec(url: URL(string: raw_url)!, sha: sha, sortIndex: sortIndex) } @@ -90,8 +96,9 @@ struct GitHubFetcher { static let githubMainBranchEndpoint = endpointBaseURL.appending(path:"branches/main") static let githubIssuesEndpoint = endpointBaseURL.appending(path: "issues?since=2023-08-01T01:00:00Z&state=all") static let githubProposalsEndpoint = endpointBaseURL.appending(path: "contents/proposals" ) - static func githubPullEndpoint(for request: String) -> URL { + static func githubPullEndpoint(for request: Int) -> URL { endpointBaseURL.appending(path: "pulls/\(request)/files") + .appending(queryItems: [URLQueryItem(name: "per_page", value: "100")]) } } @@ -126,11 +133,10 @@ struct GitHubFetcher { return try await getGitHubAPIValue(for: endpoint, type: [GitHubContentItem].self).filter { $0.isMarkdownFile } } - static func fetchPullRequestProposalList(for pullNumber: String) async throws -> [GitHubPullFileItem] { + static func fetchPullRequestProposalList(for pullNumber: Int) async throws -> [GitHubPullFileItem] { let endpointURL = Endpoint.githubPullEndpoint(for: pullNumber) let contents = try await getGitHubAPIValue(for: endpointURL, type: [GitHubPullFileItem].self) - return contents - .filter { $0.filename.hasPrefix("proposals/") } + return contents.filter { $0.isProposalFile } } static func getGitHubAPIValue(for endpoint: URL, type: T.Type, cachePolicy: URLRequest.CachePolicy = .useProtocolCachePolicy) async throws -> T { diff --git a/Sources/swift-evolution-metadata-extractor/Help.swift b/Sources/swift-evolution-metadata-extractor/Help.swift index 27aef8b..fb81edd 100644 --- a/Sources/swift-evolution-metadata-extractor/Help.swift +++ b/Sources/swift-evolution-metadata-extractor/Help.swift @@ -91,7 +91,13 @@ enum Help { static let discussion = """ Running with no arguments will read from the swift-evolution repository, extract metadata and write a validation report to stdout. + To validate the proposal files in a pull request, use the --pull-request option with the pull number. + If validation errors are found the process will exit with error code 1. """ + + enum Argument { + static let pullRequest: ArgumentHelp = "Pull request number." + } } } diff --git a/Sources/swift-evolution-metadata-extractor/ValidateCommand.swift b/Sources/swift-evolution-metadata-extractor/ValidateCommand.swift index 97208d2..559abae 100644 --- a/Sources/swift-evolution-metadata-extractor/ValidateCommand.swift +++ b/Sources/swift-evolution-metadata-extractor/ValidateCommand.swift @@ -28,13 +28,16 @@ struct ValidateCommand: AsyncParsableCommand { @Flag(name: .shortAndLong, help: Help.Shared.Argument.verbose) var verbose: Bool = false + @Option(name: .long, help: Help.Validate.Argument.pullRequest) + var pullRequest: Int? + @Argument(help: Help.Shared.Argument.proposalFiles, transform: ArgumentValidation.proposalURL) var proposalURLs: [URL] = [] mutating func validate() throws { ArgumentValidation.validate(verbose: verbose) ArgumentValidation.validateHTTPProxies() - extractionSource = try ArgumentValidation.extractionSource(snapshotURL: snapshotURL, proposalURLs: proposalURLs) + extractionSource = try ArgumentValidation.extractionSource(snapshotURL: snapshotURL, proposalURLs: proposalURLs, pullRequest: pullRequest) }