Repository navigation
fix: compile-check's summary says 1 assembly and 1 error #3334
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
hatayama
merged 3 commits into
feature/compile-check-changed-files-check
from
fix/compile-check-message-plurals
Oct 15, 2026
Merged
Changes from all commits
Commits
Show all changes
3 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
84 changes: 84 additions & 0 deletions
84
cli/dispatcher/internal/dispatcher/compile_check_message.go
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,84 @@ | ||
| package dispatcher | ||
|
|
||
| import ( | ||
| "fmt" | ||
| "strings" | ||
| ) | ||
|
|
||
| // compileCheckMessage states what the run did in one line. | ||
| // Why the blocked assemblies are added rather than folded in: without them a run that reports one | ||
| // error and hundreds of assemblies it never got to reads as a run that lost them. | ||
| // Why the fast-checked ones get a reminder: the check skipped rules only a full compile runs, and | ||
| // the caller has to know to run --full before finishing. | ||
| func compileCheckMessage(response compileCheckResponse) string { | ||
| message := compileCheckRunSummary(response) | ||
| if len(response.BlockedAssemblies) > 0 { | ||
| count := len(response.BlockedAssemblies) | ||
| message += fmt.Sprintf(compileCheckPlural(count, | ||
| " %d assembly was not compiled because an assembly it references has errors.", | ||
| " %d assemblies were not compiled because an assembly they reference has errors."), count) | ||
| } | ||
| if len(response.FastCheckedAssemblies) > 0 { | ||
| count := len(response.FastCheckedAssemblies) | ||
| message += fmt.Sprintf(compileCheckPlural(count, | ||
| " %d assembly was checked on its changed files only;", | ||
| " %d assemblies were checked on their changed files only;"), count) + | ||
| " run with --full before finishing to cover assembly-wide rules." | ||
| } | ||
|
|
||
| return message | ||
| } | ||
|
|
||
| // compileCheckRunSummary states what the run compiled, reused, and fast-checked. | ||
| // Why only the non-zero counts are named: a run that reused nothing reads exactly as it did before | ||
| // reuse existed, and a run that compiled nothing should not open by saying so. | ||
| func compileCheckRunSummary(response compileCheckResponse) string { | ||
| counts := []struct { | ||
| verb string | ||
| count int | ||
| }{ | ||
| {"compiled", len(response.CompiledAssemblies)}, | ||
| {"reused", len(response.ReusedAssemblies)}, | ||
| {"fast-checked", len(response.FastCheckedAssemblies)}, | ||
| } | ||
| parts := []string{} | ||
| for _, entry := range counts { | ||
| if entry.count == 0 { | ||
| continue | ||
| } | ||
| if len(parts) == 0 { | ||
| parts = append(parts, fmt.Sprintf("%s%s %d %s", | ||
| strings.ToUpper(entry.verb[:1]), entry.verb[1:], entry.count, | ||
| compileCheckPlural(entry.count, "assembly", "assemblies"))) | ||
| continue | ||
| } | ||
| parts = append(parts, fmt.Sprintf("%s %d", entry.verb, entry.count)) | ||
| } | ||
| if len(parts) == 0 { | ||
| return compileCheckNoChangeMessage | ||
| } | ||
|
|
||
| return fmt.Sprintf("%s with %d %s and %d %s.", | ||
| joinCompileCheckSummaryParts(parts), | ||
| response.ErrorCount, compileCheckPlural(response.ErrorCount, "error", "errors"), | ||
| response.WarningCount, compileCheckPlural(response.WarningCount, "warning", "warnings")) | ||
| } | ||
|
|
||
| // compileCheckPlural picks the wording for count, which is singular only for exactly one; zero | ||
| // takes the plural ("0 errors"). | ||
| func compileCheckPlural(count int, singular string, plural string) string { | ||
| if count == 1 { | ||
| return singular | ||
| } | ||
|
|
||
| return plural | ||
| } | ||
|
|
||
| // joinCompileCheckSummaryParts joins the summary parts as "A and b" or "A, b and c". | ||
| func joinCompileCheckSummaryParts(parts []string) string { | ||
| if len(parts) == 1 { | ||
| return parts[0] | ||
| } | ||
|
|
||
| return strings.Join(parts[:len(parts)-1], ", ") + " and " + parts[len(parts)-1] | ||
| } |
107 changes: 107 additions & 0 deletions
107
cli/dispatcher/internal/dispatcher/compile_check_message_test.go
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,107 @@ | ||
| package dispatcher | ||
|
|
||
| import ( | ||
| "strconv" | ||
| "testing" | ||
| ) | ||
|
|
||
| const compileCheckFastCheckedReminder = "run with --full before finishing to cover assembly-wide rules." | ||
|
|
||
| // compileCheckMessageNames returns count distinct assembly names, since the summary counts names. | ||
| func compileCheckMessageNames(count int) []string { | ||
| names := []string{} | ||
| for index := range count { | ||
| names = append(names, "Assembly"+strconv.Itoa(index)) | ||
| } | ||
|
|
||
| return names | ||
| } | ||
|
|
||
| // Verifies that every count the summary puts before a noun uses the singular for exactly one and | ||
| // the plural for zero and for more than one. | ||
| func TestCompileCheckMessageUsesTheSingularOnlyForACountOfOne(t *testing.T) { | ||
| cases := []struct { | ||
| name string | ||
| response compileCheckResponse | ||
| want string | ||
| }{ | ||
| { | ||
| name: "no compiled assembly leaves the first noun to the next count", | ||
| response: compileCheckResponse{ReusedAssemblies: compileCheckMessageNames(2)}, | ||
| want: "Reused 2 assemblies with 0 errors and 0 warnings.", | ||
| }, | ||
| { | ||
| name: "one compiled assembly", | ||
| response: compileCheckResponse{CompiledAssemblies: compileCheckMessageNames(1)}, | ||
| want: "Compiled 1 assembly with 0 errors and 0 warnings.", | ||
| }, | ||
| { | ||
| name: "two compiled assemblies", | ||
| response: compileCheckResponse{CompiledAssemblies: compileCheckMessageNames(2)}, | ||
| want: "Compiled 2 assemblies with 0 errors and 0 warnings.", | ||
| }, | ||
| { | ||
| name: "one error", | ||
| response: compileCheckResponse{CompiledAssemblies: compileCheckMessageNames(2), ErrorCount: 1}, | ||
| want: "Compiled 2 assemblies with 1 error and 0 warnings.", | ||
| }, | ||
| { | ||
| name: "two errors", | ||
| response: compileCheckResponse{CompiledAssemblies: compileCheckMessageNames(2), ErrorCount: 2}, | ||
| want: "Compiled 2 assemblies with 2 errors and 0 warnings.", | ||
| }, | ||
| { | ||
| name: "one warning", | ||
| response: compileCheckResponse{CompiledAssemblies: compileCheckMessageNames(2), WarningCount: 1}, | ||
| want: "Compiled 2 assemblies with 0 errors and 1 warning.", | ||
| }, | ||
| { | ||
| name: "two warnings", | ||
| response: compileCheckResponse{CompiledAssemblies: compileCheckMessageNames(2), WarningCount: 2}, | ||
| want: "Compiled 2 assemblies with 0 errors and 2 warnings.", | ||
| }, | ||
| { | ||
| name: "one blocked assembly", | ||
| response: compileCheckResponse{ | ||
| CompiledAssemblies: compileCheckMessageNames(2), | ||
| BlockedAssemblies: compileCheckMessageNames(1), | ||
| }, | ||
| want: "Compiled 2 assemblies with 0 errors and 0 warnings." + | ||
| " 1 assembly was not compiled because an assembly it references has errors.", | ||
| }, | ||
| { | ||
| name: "two blocked assemblies", | ||
| response: compileCheckResponse{ | ||
| CompiledAssemblies: compileCheckMessageNames(2), | ||
| BlockedAssemblies: compileCheckMessageNames(2), | ||
| }, | ||
| want: "Compiled 2 assemblies with 0 errors and 0 warnings." + | ||
| " 2 assemblies were not compiled because an assembly they reference has errors.", | ||
| }, | ||
| { | ||
| name: "one fast-checked assembly", | ||
| response: compileCheckResponse{ | ||
| CompiledAssemblies: compileCheckMessageNames(2), | ||
| FastCheckedAssemblies: compileCheckMessageNames(1), | ||
| }, | ||
| want: "Compiled 2 assemblies and fast-checked 1 with 0 errors and 0 warnings." + | ||
| " 1 assembly was checked on its changed files only; " + compileCheckFastCheckedReminder, | ||
| }, | ||
| { | ||
| name: "two fast-checked assemblies", | ||
| response: compileCheckResponse{ | ||
| CompiledAssemblies: compileCheckMessageNames(2), | ||
| FastCheckedAssemblies: compileCheckMessageNames(2), | ||
| }, | ||
| want: "Compiled 2 assemblies and fast-checked 2 with 0 errors and 0 warnings." + | ||
| " 2 assemblies were checked on their changed files only; " + compileCheckFastCheckedReminder, | ||
| }, | ||
| } | ||
| for _, testCase := range cases { | ||
| t.Run(testCase.name, func(t *testing.T) { | ||
| if got := compileCheckMessage(testCase.response); got != testCase.want { | ||
| t.Fatalf("unexpected summary\n got: %q\nwant: %q", got, testCase.want) | ||
| } | ||
| }) | ||
| } | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
State when the baseline is recorded.
Line 64 says the full compile records a baseline in every listed case. If that compile still has errors, it records no baseline, and the assembly continues to compile in full (see Lines 160–162). Clarify that only a successful full compile records the baseline.
🤖 Prompt for AI Agents