Skip to content
Open
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -14,3 +14,4 @@ DerivedData/

# Xcode 11
.swiftpm/
RealTests/
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this needed?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

11 changes: 11 additions & 0 deletions Sources/XCLogParser/commands/CommandHandler.swift
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,17 @@
let reporterOutput = ReporterOutputFactory.makeReporterOutput(path: options.outputPath)
let logReporter = options.reporter.makeLogReporter()
try logReporter.report(build: buildSteps, output: reporterOutput, rootOutput: options.rootOutput)

Check failure on line 74 in Sources/XCLogParser/commands/CommandHandler.swift

View workflow job for this annotation

GitHub Actions / SwiftLint

Trailing Whitespace Violation: Lines should not have trailing whitespace. (trailing_whitespace)
// Exit with appropriate code based on build status
if shouldExitWithFailureCode(buildSteps: buildSteps) {
exit(1)
}
}

Check failure on line 80 in Sources/XCLogParser/commands/CommandHandler.swift

View workflow job for this annotation

GitHub Actions / SwiftLint

Trailing Whitespace Violation: Lines should not have trailing whitespace. (trailing_whitespace)
private func shouldExitWithFailureCode(buildSteps: BuildStep) -> Bool {
Copy link

Copilot AI Jun 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[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.

Copilot uses AI. Check for mistakes.

// Check if the main build failed
let buildStatus = buildSteps.buildStatus.lowercased()
return buildStatus.contains("failed") || buildStatus.contains("error")
}

}
2 changes: 2 additions & 0 deletions Sources/XCLogParser/parser/BuildStep.swift
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,8 @@ public enum DetailStepType: String, Encodable {
return .cCompilation
case Prefix("CompileSwift "):
return .swiftCompilation
case Prefix("SwiftCompile "):
return .swiftCompilation
case Prefix("Ld "):
return .linker
case Prefix("PhaseScriptExecution "):
Expand Down
24 changes: 18 additions & 6 deletions Sources/XCLogParser/parser/Notice+Parser.swift
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,8 @@
/// - returns: An Array of `Notice`
public static func parseFromLogSection(_ logSection: IDEActivityLogSection,
forType type: DetailStepType,
truncLargeIssues: Bool)
truncLargeIssues: Bool,
isSubsection: Bool = false)
-> [Notice] {
var logSection = logSection
if truncLargeIssues && logSection.messages.count > 100 {
Expand Down Expand Up @@ -60,7 +61,9 @@
// Special case, Interface builder warning can only be spotted by checking the whole text of the
// log section
let noticeTypeTitle = message.categoryIdent.isEmpty ? logSection.text : message.categoryIdent
if var notice = Notice(withType: NoticeType.fromTitle(noticeTypeTitle),
let initialType = NoticeType.fromTitleAndSeverity(noticeTypeTitle, severity: message.severity)

Check failure on line 65 in Sources/XCLogParser/parser/Notice+Parser.swift

View workflow job for this annotation

GitHub Actions / SwiftLint

Trailing Whitespace Violation: Lines should not have trailing whitespace. (trailing_whitespace)
if var notice = Notice(withType: initialType,
logMessage: message,
detail: logSection.text) {
// Add the right details to Swift errors
Expand All @@ -73,8 +76,8 @@
var errorLocation = notice.documentURL.replacingOccurrences(of: "file://", with: "")
errorLocation += ":\(notice.startingLineNumber):\(notice.startingColumnNumber):"
// do not report error in a file that it does not belong to (we'll ended
// up having duplicated errors)
if !logSection.location.documentURLString.isEmpty
// up having duplicated errors) - but only for main sections, not subsections
if !isSubsection && !logSection.location.documentURLString.isEmpty
&& logSection.location.documentURLString != notice.documentURL {
return nil
}
Expand Down Expand Up @@ -164,8 +167,17 @@
}
return zip(logSection.messages, clangFlags)
.compactMap { (message, warningFlag) -> Notice? in
// If the warning is treated as error, we marked the issue as error
let type: NoticeType = warningFlag.contains("-Werror") ? .clangError : .clangWarning
// Determine type based on both flags and severity
var type: NoticeType
if warningFlag.contains("-Werror") {
type = .clangError
} else {
// Use severity to determine if this should be treated as error or warning
// Severity 2+ = error (treated as error by compiler)
// Severity 1 = warning
type = message.severity >= 2 ? .clangError : .clangWarning
}

Check failure on line 180 in Sources/XCLogParser/parser/Notice+Parser.swift

View workflow job for this annotation

GitHub Actions / SwiftLint

Trailing Whitespace Violation: Lines should not have trailing whitespace. (trailing_whitespace)
let notice = Notice(withType: type, logMessage: message, clangFlag: warningFlag)

if let notice = notice,
Expand Down
40 changes: 40 additions & 0 deletions Sources/XCLogParser/parser/NoticeType.swift
Original file line number Diff line number Diff line change
Expand Up @@ -94,8 +94,48 @@
return .swiftError
case Suffix("failed with a nonzero exit code"):
return .failedCommandError
case "No-usage":
return .swiftWarning
default:
return .note
}
}

Check failure on line 103 in Sources/XCLogParser/parser/NoticeType.swift

View workflow job for this annotation

GitHub Actions / SwiftLint

Trailing Whitespace Violation: Lines should not have trailing whitespace. (trailing_whitespace)
/// Returns a NoticeType based on both categoryIdent title and severity level
/// This method handles ambiguous categories that can be either warnings or errors
/// Severity levels: 0 = note, 1 = warning, 2+ = error (treated as error by compiler)
public static func fromTitleAndSeverity(_ title: String, severity: Int) -> NoticeType? {

Check failure on line 107 in Sources/XCLogParser/parser/NoticeType.swift

View workflow job for this annotation

GitHub Actions / SwiftLint

Cyclomatic Complexity Violation: Function should have complexity 10 or less: currently complexity equals 12 (cyclomatic_complexity)
// First get the initial type from title
guard let initialType = fromTitle(title) else {
return nil
}

Check failure on line 112 in Sources/XCLogParser/parser/NoticeType.swift

View workflow job for this annotation

GitHub Actions / SwiftLint

Trailing Whitespace Violation: Lines should not have trailing whitespace. (trailing_whitespace)
// Use severity to override type classification when needed
switch severity {
case 0:
return .note
case 1:
switch initialType {
case .clangError:
return .clangWarning
case .swiftError:
return .swiftWarning
default:
return initialType
}
case 2...:
switch initialType {
case .clangWarning, .projectWarning:
return .clangError
case .swiftWarning:
return .swiftError
case .note:
return .error
default:
return initialType
}
default:
return initialType
}
}
}
78 changes: 75 additions & 3 deletions Sources/XCLogParser/parser/ParserBuildSteps.swift
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,31 @@
self.truncLargeIssues = truncLargeIssues
}

/// 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]) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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]) {

var allWarnings: [Notice] = []
var allErrors: [Notice] = []

func collectNotices(from step: BuildStep) {
if let warnings = step.warnings {
allWarnings.append(contentsOf: warnings)
}
if let errors = step.errors {
allErrors.append(contentsOf: errors)
}

for subStep in step.subSteps {
collectNotices(from: subStep)
}
}

collectNotices(from: buildStep)

return (allWarnings.removingDuplicates(), allErrors.removingDuplicates())
}

/// Parses the content from an Xcode log into a `BuildStep`
/// - parameter activityLog: An `IDEActivityLog`
/// - returns: A `BuildStep` with the parsed content from the log.
Expand All @@ -106,8 +131,12 @@
buildStatus = BuildStatusSanitizer.sanitize(originalStatus: activityLog.mainSection.localizedResultString)
let mainSectionWithTargets = activityLog.mainSection.groupedByTarget()
var mainBuildStep = try parseLogSection(logSection: mainSectionWithTargets, type: .main, parentSection: nil)
mainBuildStep.errorCount = totalErrors
mainBuildStep.warningCount = totalWarnings

// Collect and deduplicate all warnings and errors from the entire build tree
let (deduplicatedWarnings, deduplicatedErrors) = collectAndDeduplicateNotices(from: mainBuildStep)
mainBuildStep.errorCount = deduplicatedErrors.count
mainBuildStep.warningCount = deduplicatedWarnings.count

mainBuildStep = decorateWithSwiftcTimes(mainBuildStep)
return mainBuildStep
}
Expand All @@ -131,7 +160,41 @@
targetErrors = 0
targetWarnings = 0
}
let notices = parseWarningsAndErrorsFromLogSection(logSection, forType: detailType)
var notices = parseWarningsAndErrorsFromLogSection(logSection, forType: detailType)


// For Swift compilations and other compilation types, also check subsections for errors/warnings
// Also ensure Swift file-level compilations are processed correctly
if (detailType == .swiftCompilation || detailType == .cCompilation || detailType == .other) && !logSection.subSections.isEmpty {

Check failure on line 168 in Sources/XCLogParser/parser/ParserBuildSteps.swift

View workflow job for this annotation

GitHub Actions / SwiftLint

Line Length Violation: Line should be 120 characters or less: currently 140 characters (line_length)
// Initialize notices if nil
if notices == nil {
notices = ["warnings": [], "errors": [], "notes": []]
}

for subSection in logSection.subSections {
if let subNotices = parseWarningsAndErrorsFromLogSectionAsSubsection(subSection, forType: detailType) {

Check failure on line 175 in Sources/XCLogParser/parser/ParserBuildSteps.swift

View workflow job for this annotation

GitHub Actions / SwiftLint

Line Length Violation: Line should be 120 characters or less: currently 123 characters (line_length)
// Merge subsection notices with parent section notices
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
}
Comment on lines +177 to +193
Copy link

Copilot AI Jun 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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")

Copilot uses AI. Check for mistakes.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

}
}
}

let warnings: [Notice]? = notices?["warnings"]
let errors: [Notice]? = notices?["errors"]
let notes: [Notice]? = notices?["notes"]
Expand All @@ -146,6 +209,7 @@
totalWarnings += warnings.count
targetWarnings += warnings.count
}

var step = BuildStep(type: type,
machineName: machineName,
buildIdentifier: self.buildIdentifier,
Expand Down Expand Up @@ -310,6 +374,14 @@
"errors": notices.getErrors(),
"notes": notices.getNotes()]
}

private func parseWarningsAndErrorsFromLogSectionAsSubsection(_ logSection: IDEActivityLogSection, forType type: DetailStepType)
-> [String: [Notice]]? {
let notices = Notice.parseFromLogSection(logSection, forType: type, truncLargeIssues: truncLargeIssues, isSubsection: true)
return ["warnings": notices.getWarnings(),
"errors": notices.getErrors(),
"notes": notices.getNotes()]
}

private func decorateWithSwiftcTimes(_ mainStep: BuildStep) -> BuildStep {
swiftCompilerParser.parse()
Expand Down Expand Up @@ -395,4 +467,4 @@
andCompilationDuration: lastStep.compilationEndTimestamp - app.startTimestamp)
}

}

Check failure on line 470 in Sources/XCLogParser/parser/ParserBuildSteps.swift

View workflow job for this annotation

GitHub Actions / SwiftLint

File Line Length Violation: File should contain 400 lines or less: currently contains 470 (file_length)
Loading
Loading