Fix C++ warnings incorrectly classified as errors #231

lapfelix · 2025-06-08T01:46:35Z

C++ warnings with severity 1 were being classified as errors based solely on categoryIdent (e.g. "Parse Issue"), ignoring the severity field. Now uses severity to distinguish warnings (severity 1) from errors (severity 2+) for all ambiguous categories.

Added a test to cover this case.

To cover these warnings (which Xcode considered warnings in my build):

C++ warnings with severity 1 were being classified as errors based solely on categoryIdent (e.g. "Parse Issue"), ignoring the severity field. Now uses severity to distinguish warnings (severity 1) from errors (severity 2+) for all ambiguous categories. Signed-off-by: lapfelix <[email protected]>

- Add support for 'SwiftCompile' signature pattern in BuildStep.swift - Enhance subsection parsing in ParserBuildSteps.swift to check Swift compilation subsections for errors/warnings - Relax location filtering in Notice+Parser.swift to allow errors from subsections with different document URLs - Add support for 'No-usage' warnings to be correctly classified as Swift warnings - Add RealTests directory with comprehensive testing framework for real build logs - Test suite now correctly detects errors and warnings that were previously missed Fixes issues where: 1. Swift compilation errors in subsections were not detected (e.g. type not found errors) 2. Swift warnings like unused variables were classified as notes instead of warnings 3. Different Swift compilation signature patterns were not recognized 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>

…ions - Classify high-severity (2+) notes as errors in NoticeType.fromTitleAndSeverity() - Fixes validation mismatches where Update Signing logs expected 2 errors but XCLogParser reported 0 - Provisioning errors like "No Account for Team" were being classified as notes despite severity 2+ 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>

…uracy - Add collectAndDeduplicateNotices() method to remove duplicate warnings/errors across build tree - Fixes validation mismatches where identical warnings in multiple build targets were counted separately - Improved validation accuracy from 67% to 91% (24 percentage point improvement) 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>

Copilot

Pull Request Overview

This PR fixes the misclassification of C++ warnings as errors by incorporating severity into the log message parsing logic. Key changes include:

Updated tests in ParserTests.swift to verify that messages with severity 1 are handled as warnings and those with severity 2+ as errors.
Enhanced parsing logic in ParserBuildSteps.swift and Notice+Parser.swift to deduplicate and correctly merge notices, along with adjustments in NoticeType.swift and BuildStep.swift.
Added exit-code handling in CommandHandler.swift based on the build status.

Reviewed Changes

Copilot reviewed 6 out of 7 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
Tests/XCLogParserTests/ParserTests.swift	Added tests for ambiguous category message classification and Swift compilation error in subsections.
Sources/XCLogParser/parser/ParserBuildSteps.swift	Updated notice deduplication and merge logic to account for subsection notices.
Sources/XCLogParser/parser/NoticeType.swift	Added severity-based classification logic and a new case for “No-usage”.
Sources/XCLogParser/parser/Notice+Parser.swift	Adjusted notice creation to utilize severity for determining notice type.
Sources/XCLogParser/parser/BuildStep.swift	Added support for the new “SwiftCompile” prefix in build step type detection.
Sources/XCLogParser/commands/CommandHandler.swift	Introduced exit-code handling to signal build failures appropriately.

Copilot · 2025-06-25T15:11:03Z

Sources/XCLogParser/parser/ParserBuildSteps.swift

+                        if let parentWarnings = notices?["warnings"], let subWarnings = subNotices["warnings"] {
+                            notices?["warnings"] = parentWarnings + subWarnings
+                        } else if let subWarnings = subNotices["warnings"] {
+                            notices?["warnings"] = subWarnings
+                        }
+
+                        if let parentErrors = notices?["errors"], let subErrors = subNotices["errors"] {
+                            notices?["errors"] = parentErrors + subErrors
+                        } else if let subErrors = subNotices["errors"] {
+                            notices?["errors"] = subErrors
+                        }
+
+                        if let parentNotes = notices?["notes"], let subNotes = subNotices["notes"] {
+                            notices?["notes"] = parentNotes + subNotes
+                        } else if let subNotes = subNotices["notes"] {
+                            notices?["notes"] = subNotes
+                        }


The merging logic for 'warnings', 'errors', and 'notes' is repeated for each key. Consider extracting this duplication into a helper function to improve code maintainability.

Suggested change

if let parentWarnings = notices?["warnings"], let subWarnings = subNotices["warnings"] {

notices?["warnings"] = parentWarnings + subWarnings

} else if let subWarnings = subNotices["warnings"] {

notices?["warnings"] = subWarnings

}

if let parentErrors = notices?["errors"], let subErrors = subNotices["errors"] {

notices?["errors"] = parentErrors + subErrors

} else if let subErrors = subNotices["errors"] {

notices?["errors"] = subErrors

}

if let parentNotes = notices?["notes"], let subNotes = subNotices["notes"] {

notices?["notes"] = parentNotes + subNotes

} else if let subNotes = subNotices["notes"] {

notices?["notes"] = subNotes

}

notices = mergeNotices(parentNotices: notices, subNotices: subNotices, key: "warnings")

notices = mergeNotices(parentNotices: notices, subNotices: subNotices, key: "errors")

notices = mergeNotices(parentNotices: notices, subNotices: subNotices, key: "notes")

This is a good advice. I'd also extract the logic into a method since the method is getting very deeply nested.

Copilot · 2025-06-25T15:11:04Z

Sources/XCLogParser/commands/CommandHandler.swift

+        }
+    }
+
+    private func shouldExitWithFailureCode(buildSteps: BuildStep) -> Bool {


[nitpick] The exit logic based on a lowercased build status string may be brittle if more status values are introduced. Consider clarifying or centralizing build status definitions to make the check more robust.

lapfelix · 2025-06-25T15:12:22Z

Oh woops I forgot I had this PR open and kept committing more fixes I needed to my master branch

pepicrft

This looks good to me. I'm new to the repository though, so I'd appreciate an extra 👀 . @AvdLee perhaps?

pepicrft · 2025-06-25T15:13:24Z

Sources/XCLogParser/parser/ParserBuildSteps.swift

+    /// Collects all warnings and errors from a BuildStep tree and deduplicates them
+    /// - parameter buildStep: The root BuildStep to collect notices from
+    /// - returns: A tuple of (warnings, errors) arrays with duplicates removed
+    private func collectAndDeduplicateNotices(from buildStep: BuildStep) -> ([Notice], [Notice]) {


It's not obvious from the caller what the items in the tuple represent:

Suggested change

private func collectAndDeduplicateNotices(from buildStep: BuildStep) -> ([Notice], [Notice]) {

private func collectAndDeduplicateNotices(from buildStep: BuildStep) -> (warnings: [Notice], errors: [Notice]) {

pepicrft · 2025-06-25T15:14:07Z

Sources/XCLogParser/parser/ParserBuildSteps.swift

+                        if let parentWarnings = notices?["warnings"], let subWarnings = subNotices["warnings"] {
+                            notices?["warnings"] = parentWarnings + subWarnings
+                        } else if let subWarnings = subNotices["warnings"] {
+                            notices?["warnings"] = subWarnings
+                        }
+
+                        if let parentErrors = notices?["errors"], let subErrors = subNotices["errors"] {
+                            notices?["errors"] = parentErrors + subErrors
+                        } else if let subErrors = subNotices["errors"] {
+                            notices?["errors"] = subErrors
+                        }
+
+                        if let parentNotes = notices?["notes"], let subNotes = subNotices["notes"] {
+                            notices?["notes"] = parentNotes + subNotes
+                        } else if let subNotes = subNotices["notes"] {
+                            notices?["notes"] = subNotes
+                        }


This is a good advice. I'd also extract the logic into a method since the method is getting very deeply nested.

pepicrft · 2025-06-25T15:15:00Z

.gitignore


 # Xcode 11
 .swiftpm/
+RealTests/


Is this needed?

No, I'll remove it. What I did is I gathered a few hundred xcactivitylogs from DerivedData, put them in that RealTests folder, along with their LogStoreManifest.plist. Then had a script that ran XCLogParser on them and compared the # of warnings and errors. I then iterated on that to get it to match as much as possible. I'd need to check the xcactivitylogs to see if there's private info in there (like my global env vars). It started with 80% match and I got to 99.1% match.

I haven't manually reviewed the code yet like I did for this PR (I had Claude Code iterate on that loop to increase the % of matching), but I just pushed the changes done to reach 99.1% on my local xcactivitylogs here: master...lapfelix:XCLogParser:feature/fine-tuned-warnings-to-match-xcode

I can review and clean up what's in there, but I do think there's value in testing real xcactivitylogs against what's in LogStoreManifest.plist.

No, I'll remove it. What I did is I gathered a few hundred xcactivitylogs from DerivedData, put them in that RealTests folder, along with their LogStoreManifest.plist. Then had a script that ran XCLogParser on them and compared the # of warnings and errors. I then iterated on that to get it to match as much as possible. I'd need to check the xcactivitylogs to see if there's private info in there (like my global env vars). It started with 80% match and I got to 99.1% match.

If we just had a standardized machine-readable format from Apple... 🤦🏼

I haven't manually reviewed the code yet like I did for this PR (I had Claude Code iterate on that loop to increase the % of matching), but I just pushed the changes done to reach 99.1% on my local xcactivitylogs here: master...lapfelix:XCLogParser:feature/fine-tuned-warnings-to-match-xcode

That sounds fair! I trust you. I'd do a bit of clean up of the code and get an extra review and I'd say we are ready to merge.

AvdLee

Looks good to me, great job!

AvdLee · 2025-09-11T09:09:34Z

@lapfelix I'm ready to merge this in, but the checks are failing. Would you see a moment to check and solve accordingly? 🙏

lapfelix requested review from CognitiveDisson, aleksandergrzyb, ecamacho and polac24 as code owners June 8, 2025 01:46

lapfelix force-pushed the master branch from 2f942e7 to b2b3a4d Compare June 9, 2025 00:26

lapfelix force-pushed the master branch from b2b3a4d to 667aae3 Compare June 9, 2025 00:27

lapfelix and others added 3 commits June 21, 2025 21:45

Fix more severity type inconsistencies

36d6989

Support more Swift compilation errors

ac2d161

aleksandergrzyb requested review from AvdLee and pepicrft June 23, 2025 12:39

lapfelix and others added 2 commits June 23, 2025 14:00

pepicrft requested a review from Copilot June 25, 2025 15:10

Copilot AI reviewed Jun 25, 2025

View reviewed changes

pepicrft approved these changes Jun 25, 2025

View reviewed changes

AvdLee approved these changes Jul 14, 2025

View reviewed changes

Merge branch 'master' into master

e9ff458

	private func collectAndDeduplicateNotices(from buildStep: BuildStep) -> ([Notice], [Notice]) {
	private func collectAndDeduplicateNotices(from buildStep: BuildStep) -> (warnings: [Notice], errors: [Notice]) {

Fix C++ warnings incorrectly classified as errors #231

Are you sure you want to change the base?

Fix C++ warnings incorrectly classified as errors #231

Uh oh!

Conversation

lapfelix commented Jun 8, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI Jun 25, 2025

Choose a reason for hiding this comment

Uh oh!

pepicrft Jun 25, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jun 25, 2025

Choose a reason for hiding this comment

Uh oh!

lapfelix commented Jun 25, 2025

Uh oh!

pepicrft left a comment

Choose a reason for hiding this comment

Uh oh!

pepicrft Jun 25, 2025

Choose a reason for hiding this comment

Uh oh!

pepicrft Jun 25, 2025

Choose a reason for hiding this comment

Uh oh!

pepicrft Jun 25, 2025

Choose a reason for hiding this comment

Uh oh!

lapfelix Jun 25, 2025

Choose a reason for hiding this comment

Uh oh!

lapfelix Jun 25, 2025

Choose a reason for hiding this comment

Uh oh!

pepicrft Jun 25, 2025

Choose a reason for hiding this comment

Uh oh!

AvdLee left a comment

Choose a reason for hiding this comment

Uh oh!

AvdLee commented Sep 11, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants