Improve error feedback and prevent duplicate entries by MartinP7r · Pull Request #44 · MartinP7r/AckGen

MartinP7r · 2026-03-24T22:08:11Z

Summary

Improves CLI output quality for Xcode build phase usage.

Changes

Error Tracking

Skipped files are collected and summarized instead of inline warnings
Count is always printed; per-file details only in debug builds (#if DEV)
DEV compilation condition defined for debug config in Package.swift

Duplicate Prevention

Breaks after first LICENSE file found per package (prevents entries when both LICENSE and LICENSE.md exist)

Success Message

Now shows count: "✓ Generated 12 acknowledgement(s) at: path"

Testing

swift build   # ✓ Builds
swift test    # ✓ 10/10 tests pass

Stacked On

This PR is stacked on #32 → #31

- Extract LicenseScanner struct for testable scanning logic - Track skipped files and print summary (verbose details behind #if DEV) - Break after first LICENSE found per package to prevent duplicates - Show acknowledgement count in success message - Add 7 LicenseScannerTests covering: file discovery, LICENSE variants, hidden dirs, duplicate prevention, invalid UTF-8 tracking, empty dirs

Copilot

Pull request overview

Improves AckGen’s CLI build-phase output by extracting license scanning into a dedicated helper, summarizing skipped files, and preventing duplicate acknowledgements when multiple LICENSE variants exist in a package.

Changes:

Added LicenseScanner with structured scan results (acknowledgements + skipped file metadata) and updated CLI output to summarize encoding skips.
Prevented duplicate entries by breaking after the first matching LICENSE variant per package.
Added DEV Swift compilation condition (debug-only) and introduced new unit tests for license scanning behavior.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.

File	Description
Sources/AckGenCLI/AckGen.swift	Introduces `LicenseScanner`, summarizes skipped-file warnings, and updates success output to include generated count.
Tests/AckGenTests/LicenseScannerTests.swift	Adds test coverage for license scanning scenarios (variants, hidden dirs, duplicates, invalid UTF-8).
Package.swift	Defines `DEV` for debug builds of `AckGenCLI` and updates test target dependencies to include `AckGenCLI`.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-03-25T01:51:48Z

Sources/AckGenCLI/AckGen.swift

+    static let licenseFiles = ["LICENSE", "LICENSE.txt", "LICENSE.md"]
+
+    static func scan(checkoutsPath: String, fileManager: FileManager = .default) throws -> ScanResult {
+        let packageDirectories = try fileManager.contentsOfDirectory(atPath: checkoutsPath)


contentsOfDirectory(atPath:) returns entries in an unspecified order, so the generated acknowledgements array (and resulting plist) can be nondeterministic between runs/filesystems. Consider sorting packageDirectories (or result.acknowledgements via .sorted()) before encoding to keep output stable and reduce noisy diffs in build phases.

Suggested change

let packageDirectories = try fileManager.contentsOfDirectory(atPath: checkoutsPath)

let packageDirectories = try fileManager.contentsOfDirectory(atPath: checkoutsPath).sorted()

Copilot · 2026-03-25T01:51:48Z

Sources/AckGenCLI/AckGen.swift

+    struct ScanResult {
+        var acknowledgements: [Acknowledgement]
+        var skippedFiles: [(package: String, file: String, reason: String)]


skippedFiles is typed as an inline tuple here and the same tuple shape is repeated in printSkippedFilesSummary. To avoid the type drifting over time, consider introducing a dedicated SkippedFile struct (or a typealias) and reusing it in both places.

Suggested change

struct ScanResult {

var acknowledgements: [Acknowledgement]

var skippedFiles: [(package: String, file: String, reason: String)]

typealias SkippedFile = (package: String, file: String, reason: String)

struct ScanResult {

var acknowledgements: [Acknowledgement]

var skippedFiles: [SkippedFile]

Copilot · 2026-03-25T01:51:49Z

Tests/AckGenTests/LicenseScannerTests.swift

+    override func setUp() {
+        super.setUp()
+        tempDir = NSTemporaryDirectory() + "AckGenTests-\(UUID().uuidString)"
+        try! fman.createDirectory(atPath: tempDir, withIntermediateDirectories: true)
+    }
+
+    override func tearDown() {
+        try? fman.removeItem(atPath: tempDir)
+        super.tearDown()
+    }
+
+    // MARK: - Helpers
+
+    private func createPackage(_ name: String, licenseFile: String = "LICENSE", content: String = "MIT License") {
+        let pkgPath = "\(tempDir!)/\(name)"
+        try! fman.createDirectory(atPath: pkgPath, withIntermediateDirectories: true)
+        let data = content.data(using: .utf8)!


The test setup/helpers use try! and force unwraps (tempDir!, data(using:)!). If anything goes wrong (e.g. temp dir creation), this will crash the test process rather than reporting a test failure. Prefer setUpWithError() (or XCTUnwrap + XCTFail) and throwing helpers so failures are surfaced as XCTest failures instead of fatal errors.

Copilot · 2026-03-25T01:51:49Z

Tests/AckGenTests/LicenseScannerTests.swift

+        XCTAssertEqual(result.skippedFiles.first?.package, "BadEncoding")
+        XCTAssertEqual(result.skippedFiles.first?.reason, "invalid UTF-8 encoding")
+    }
+


LicenseScanner.scan has a behavior branch where the first matching license file is present but invalid UTF-8, and a later variant (e.g. LICENSE.md) is valid; this would currently record a skipped file and still generate an acknowledgement. There isn’t a test covering that mixed case—adding one would lock in the intended behavior for skippedFiles reporting.

Suggested change

func testScanRecordsSkippedInvalidFirstLicenseButUsesValidVariant() throws {

// Package has an invalid UTF-8 LICENSE but a valid LICENSE.md.

// Scanner should record the invalid file in skippedFiles and still

// produce an acknowledgement based on the valid variant.

let pkgPath = "\(tempDir!)/MixedLicense"

try fman.createDirectory(atPath: pkgPath, withIntermediateDirectories: true)

// Create an invalid UTF-8 LICENSE file.

let invalidData = Data([0xFF])

fman.createFile(atPath: "\(pkgPath)/LICENSE", contents: invalidData)

// And a valid UTF-8 LICENSE.md variant.

fman.createFile(atPath: "\(pkgPath)/LICENSE.md", contents: "MIT License".data(using: .utf8)!)

let result = try LicenseScanner.scan(checkoutsPath: tempDir)

XCTAssertEqual(result.acknowledgements.count, 1)

XCTAssertEqual(result.acknowledgements.first?.title, "MixedLicense")

XCTAssertEqual(result.skippedFiles.count, 1)

XCTAssertEqual(result.skippedFiles.first?.package, "MixedLicense")

XCTAssertEqual(result.skippedFiles.first?.reason, "invalid UTF-8 encoding")

}

Reflect new LicenseScanner struct, DEV compiler flag, unused EnvironmentKey annotation, and new LicenseScannerTests.

MartinP7r changed the base branch from feature/add-cli-tests to main March 25, 2026 01:45

MartinP7r force-pushed the feature/improved-error-feedback branch from 3b61e58 to cfec7f7 Compare March 25, 2026 01:47

MartinP7r marked this pull request as ready for review March 25, 2026 01:47

Copilot AI review requested due to automatic review settings March 25, 2026 01:47

Copilot started reviewing on behalf of MartinP7r March 25, 2026 01:48 View session

Copilot AI reviewed Mar 25, 2026

View reviewed changes

MartinP7r merged commit 669684c into main Mar 25, 2026
5 checks passed

MartinP7r added a commit that referenced this pull request Mar 25, 2026

Update CLAUDE.md for LicenseScanner extraction (#44)

29f87d8

Reflect new LicenseScanner struct, DEV compiler flag, unused EnvironmentKey annotation, and new LicenseScannerTests.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Improve error feedback and prevent duplicate entries#44

Improve error feedback and prevent duplicate entries#44
MartinP7r merged 1 commit intomainfrom
feature/improved-error-feedback

MartinP7r commented Mar 24, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Mar 25, 2026

Uh oh!

Copilot AI Mar 25, 2026

Uh oh!

Copilot AI Mar 25, 2026

Uh oh!

Copilot AI Mar 25, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

	let packageDirectories = try fileManager.contentsOfDirectory(atPath: checkoutsPath)
	let packageDirectories = try fileManager.contentsOfDirectory(atPath: checkoutsPath).sorted()

+    func testScanRecordsSkippedInvalidFirstLicenseButUsesValidVariant() throws {
+        // Package has an invalid UTF-8 LICENSE but a valid LICENSE.md.
+        // Scanner should record the invalid file in skippedFiles and still
+        // produce an acknowledgement based on the valid variant.
+        let pkgPath = "\(tempDir!)/MixedLicense"
+        try fman.createDirectory(atPath: pkgPath, withIntermediateDirectories: true)
+        // Create an invalid UTF-8 LICENSE file.
+        let invalidData = Data([0xFF])
+        fman.createFile(atPath: "\(pkgPath)/LICENSE", contents: invalidData)
+        // And a valid UTF-8 LICENSE.md variant.
+        fman.createFile(atPath: "\(pkgPath)/LICENSE.md", contents: "MIT License".data(using: .utf8)!)
+        let result = try LicenseScanner.scan(checkoutsPath: tempDir)
+        XCTAssertEqual(result.acknowledgements.count, 1)
+        XCTAssertEqual(result.acknowledgements.first?.title, "MixedLicense")
+        XCTAssertEqual(result.skippedFiles.count, 1)
+        XCTAssertEqual(result.skippedFiles.first?.package, "MixedLicense")
+        XCTAssertEqual(result.skippedFiles.first?.reason, "invalid UTF-8 encoding")
+    }

Uh oh!

Conversation

MartinP7r commented Mar 24, 2026

Summary

Changes

Error Tracking

Duplicate Prevention

Success Message

Testing

Stacked On

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Mar 25, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 25, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 25, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 25, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants