diff --git a/.claude/skills/fieldworks-code-commenting/SKILL.md b/.claude/skills/fieldworks-code-commenting/SKILL.md index 4e2c0bfd21..2fca35870d 100644 --- a/.claude/skills/fieldworks-code-commenting/SKILL.md +++ b/.claude/skills/fieldworks-code-commenting/SKILL.md @@ -22,6 +22,12 @@ A C-style `/* */` block comment is not scanned -- only whole-line rules (accuracy, WHAT-not-HOW, standalone clarity) are not mechanically checked and still apply while authoring. +In CI a violation fails nothing. Each one lands as a warning annotation on +the diff, and the whole set as a single pull request comment that updates +in place on every push. `build.ps1 -CommentHygiene` and +`test.ps1 -CommentHygiene` are what make the same violations blocking, and +an agent passes one of them on every run. + ## The standard 1. **Accuracy first, then brevity.** A wrong comment is worse than none. @@ -115,9 +121,9 @@ Place above the nesting level the code spans. **Line width is separate, and applies to every comment line**, doc comments included: no comment line may exceed `.editorconfig`'s `max_line_length` -(98 columns today), counting a tab as `tab_width` columns. The gate reads -those two values from `.editorconfig` itself, so the limit can never drift -from the one the rest of the repo follows. Enforced as +(98 columns today), counting a tab as `tab_width` columns. +`comment-hygiene` reads those two values from `.editorconfig` itself, so the +limit can never drift from the one the rest of the repo follows. Enforced as `comment-line-too-long`; a local run re-wraps the line for you, CI only reports it. @@ -125,12 +131,11 @@ reports it. introducing a region whose decision-point count reaches 10 (McCabe complexity 11 -- the classic "high" threshold) gets the larger budget automatically, because a reader there needs the invariants spelled out and -200 characters buys about two sentences. Nothing opts in by hand: the gate -measures the code the comment introduces, stopping at the end of the -enclosing block or 40 lines. This fires on roughly 2% of the comments -already over 200 characters, and is meant to stay that rare -- if a comment -in ordinary straight-line code will not fit, shorten it rather than looking -for a way to qualify. +200 characters buys about two sentences. `comment-hygiene` measures the code +the comment introduces, stopping at the end of the enclosing block or 40 +lines. This fires on roughly 2% of the comments already over 200 characters, +and is meant to stay that rare -- if a comment in ordinary straight-line code +will not fit, shorten it rather than looking for a way to qualify. **Exemptions from the length cap:** a C#/C/C++/IDL `///` doc comment; a PowerShell comment-based help block (`<# ... #>`); and, in a project file or diff --git a/.github/instructions/build.instructions.md b/.github/instructions/build.instructions.md index 7d5ca4ec65..5d24fbc1dd 100644 --- a/.github/instructions/build.instructions.md +++ b/.github/instructions/build.instructions.md @@ -23,9 +23,7 @@ FieldWorks is **Windows-first** and **x64-only**. Use the repo scripts so build - Use `.\build.ps1` for builds and `.\test.ps1` for tests. - Pass `-CommentHygiene` on every build and test run. It fails the run on any comment-hygiene violation in the lines your branch adds, so you fix your own - comments before review. Humans omit it and never see the gate; CI reports - violations as warning annotations without failing. Never drop the flag to get - a run to pass. + comments before review. Never drop the flag to get a run to pass. - Avoid ad-hoc `msbuild`/`dotnet build` invocations unless you are explicitly debugging build infrastructure. - Do not change COM/registry behavior without an explicit plan and tests. diff --git a/.github/instructions/testing.instructions.md b/.github/instructions/testing.instructions.md index 894126e9a6..2393f5f302 100644 --- a/.github/instructions/testing.instructions.md +++ b/.github/instructions/testing.instructions.md @@ -17,7 +17,6 @@ Guidance for writing and running deterministic unit and integration tests for Fi Use `.\test.ps1` for all managed (C#) tests. Always pass `-CommentHygiene`: it fails the run on any comment-hygiene violation in the lines your branch adds. -Humans omit it and never see the gate; CI annotates without failing. ```powershell # Run all tests (builds first) diff --git a/.github/workflows/CI.yml b/.github/workflows/CI.yml index c1b2d5c4e5..26c4b91960 100644 --- a/.github/workflows/CI.yml +++ b/.github/workflows/CI.yml @@ -23,11 +23,6 @@ jobs: - name: Checkout Files uses: actions/checkout@v7 id: checkout - with: - # comment-hygiene.ps1 diffs against origin/; a - # shallow, single-branch checkout leaves that ref unresolvable - # and the gate fails every build with "bad revision". - fetch-depth: 0 # Real-runtime check: this repo is authored on PowerShell 7 but # build.ps1/test.ps1 run under Windows PowerShell 5.1 below. A script @@ -42,6 +37,8 @@ jobs: run: | Build/Agent/CommentHygiene.Tests.ps1 if ($LASTEXITCODE -ne 0) { exit $LASTEXITCODE } + Build/Agent/Test-BuildCommentHygieneComment.ps1 + if ($LASTEXITCODE -ne 0) { exit $LASTEXITCODE } - name: Comment hygiene fixture tests (Windows PowerShell 5.1) id: comment-hygiene-tests-winps @@ -49,6 +46,8 @@ jobs: run: | Build\Agent\CommentHygiene.Tests.ps1 if ($LASTEXITCODE -ne 0) { exit $LASTEXITCODE } + Build\Agent\Test-BuildCommentHygieneComment.ps1 + if ($LASTEXITCODE -ne 0) { exit $LASTEXITCODE } - name: Build with tests id: build diff --git a/.github/workflows/CommentHygiene.yml b/.github/workflows/CommentHygiene.yml new file mode 100644 index 0000000000..37636c612c --- /dev/null +++ b/.github/workflows/CommentHygiene.yml @@ -0,0 +1,69 @@ +name: Comment hygiene check +on: + pull_request: + +permissions: + contents: read + # pull-requests: write lets the sticky-comment steps post and clear the + # comment-hygiene comment on the PR. + pull-requests: write + +# Avoid unnecessary runs +concurrency: + group: ${{ github.workflow }}-${{ github.event.pull_request.number || github.ref }} + cancel-in-progress: true + +jobs: + # Separate from the build so the report lands on the pull request in about a + # minute instead of waiting on Debug plus tests, and still arrives when the + # build fails or is superseded. + comment_hygiene: + name: Report comment hygiene + runs-on: windows-2022 + steps: + - name: Checkout repository + uses: actions/checkout@v7 + with: + # comment-hygiene.ps1 diffs against origin/; a shallow, + # single-branch checkout leaves that ref unresolvable. + fetch-depth: 0 + + # Windows PowerShell 5.1 on windows-2022: the engine and platform the + # comment-hygiene scripts are exercised under everywhere else in CI. + - name: Scan the lines this pull request adds + shell: powershell + continue-on-error: true + run: | + .\Build\Agent\comment-hygiene.ps1 -Advisory -ReportPath .\Output\CommentHygiene\report.json + + - name: Compose the pull request comment + id: comment + shell: powershell + continue-on-error: true + env: + HEAD_SHA: ${{ github.event.pull_request.head.sha }} + run: | + .\Build\Agent\Build-CommentHygieneComment.ps1 -ReportPath .\Output\CommentHygiene\report.json + + # Advisory to the end: a comment this workflow cannot post -- a pull request + # from a fork gets a read-only token -- must not read as a failed check. + - name: Post the comment-hygiene summary + if: ${{ steps.comment.outputs.has_violations == 'true' }} + continue-on-error: true + uses: marocchino/sticky-pull-request-comment@0ea0beb66eb9baf113663a64ec522f60e49231c0 # v3.0.4 + with: + header: fieldworks-comment-hygiene + path: ${{ steps.comment.outputs.comment_path }} + skip_unchanged: true + + # only_update: a clean pull request has nothing to say, so this replaces an + # earlier summary where one exists and posts nothing where none does. + - name: Clear the comment-hygiene summary + if: ${{ steps.comment.outputs.has_violations == 'false' }} + continue-on-error: true + uses: marocchino/sticky-pull-request-comment@0ea0beb66eb9baf113663a64ec522f60e49231c0 # v3.0.4 + with: + header: fieldworks-comment-hygiene + only_update: true + path: ${{ steps.comment.outputs.comment_path }} + skip_unchanged: true diff --git a/AGENTS.md b/AGENTS.md index e7dabcfbaf..4231d2516f 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -9,9 +9,8 @@ Minimal, high-signal guidance for coding agents in this repository. - Test with `.\test.ps1 -CommentHygiene`. - `-CommentHygiene` is required of agents and not of humans: it fails the run on any comment-hygiene violation in the lines your branch adds, so you fix your - own comments before they reach review. A developer build omits it and never - runs the gate; CI reports violations as warning annotations without failing. - Do not drop the flag to get a build through. + own comments before they reach review. Do not drop the flag to get a build + through. - Do not bypass repository scripts for normal build/test work. - Commit messages must pass `gitlint` (CI: `.github/workflows/CommitMessage.yml`): title <=72 characters, body lines <=80 characters, blank line between diff --git a/Build/Agent/Build-CommentHygieneComment.ps1 b/Build/Agent/Build-CommentHygieneComment.ps1 new file mode 100644 index 0000000000..27c185ce0a --- /dev/null +++ b/Build/Agent/Build-CommentHygieneComment.ps1 @@ -0,0 +1,213 @@ +<# +.SYNOPSIS + Renders the comment-hygiene pull request comment from a scan report. + +.DESCRIPTION + Turns the report comment-hygiene.ps1 writes under -ReportPath into a markdown + body for a sticky pull request comment, and publishes comment_path and + has_violations as GitHub step outputs so the workflow can pick between posting + and clearing. Violations are listed as a table of file links, categories, and + comment text, capped at -MaxListed rows. + +.PARAMETER ReportPath + The JSON report comment-hygiene.ps1 writes. A missing file is an error, not an + empty report: a scan always writes one. + +.PARAMETER CommentPath + Where to write the markdown body. Defaults beside the report. + +.PARAMETER MaxListed + How many violations the table carries before it summarizes the rest as a + count. + +.PARAMETER Sha + Commit the file links resolve against. Pass a pull request's head commit; + links fall back to plain text when this or Repository is unset. + +.EXAMPLE + Build/Agent/Build-CommentHygieneComment.ps1 -ReportPath Output/CommentHygiene/report.json +#> +[CmdletBinding()] +param( + [string]$CommentPath, + [string]$GitHubOutputPath = $env:GITHUB_OUTPUT, + [int]$MaxListed = 25, + [string]$ReportPath, + [string]$Repository = $env:GITHUB_REPOSITORY, + [string]$RunId = $env:GITHUB_RUN_ID, + [string]$ServerUrl = $env:GITHUB_SERVER_URL, + [string]$Sha = $env:HEAD_SHA +) + +Set-StrictMode -Version Latest +$ErrorActionPreference = 'Stop' + +$repoRoot = (Resolve-Path (Join-Path $PSScriptRoot '..\..')).Path +if ([string]::IsNullOrWhiteSpace($ReportPath)) { + $ReportPath = Join-Path (Join-Path $repoRoot 'Output\CommentHygiene') 'comment-hygiene-report.json' +} +if ([string]::IsNullOrWhiteSpace($CommentPath)) { + $CommentPath = Join-Path (Split-Path -Path $ReportPath -Parent) 'comment-hygiene-comment.md' +} +if ([string]::IsNullOrWhiteSpace($ServerUrl)) { + $ServerUrl = 'https://github.com' +} +if ([string]::IsNullOrWhiteSpace($Sha)) { + $Sha = $env:GITHUB_SHA +} + +# PowerShell's current location and .NET's working directory can differ; this script +# reads through the first and writes through the second, so anchor both paths once. +if (-not [System.IO.Path]::IsPathRooted($ReportPath)) { + $ReportPath = Join-Path (Get-Location).Path $ReportPath +} +if (-not [System.IO.Path]::IsPathRooted($CommentPath)) { + $CommentPath = Join-Path (Get-Location).Path $CommentPath +} + +$SkillPath = '.claude/skills/fieldworks-code-commenting/SKILL.md' + +function Write-GitHubOutputValue { + param( + [Parameter(Mandatory = $true)] + [string]$Name, + [Parameter(Mandatory = $true)] + [string]$Value + ) + + if ([string]::IsNullOrWhiteSpace($GitHubOutputPath)) { + return + } + + # UTF8Encoding($false): a byte-order mark part way through the output file breaks + # the runner's parse of every value after it. + $encoding = New-Object System.Text.UTF8Encoding($false) + [System.IO.File]::AppendAllText($GitHubOutputPath, "$Name=$Value$([System.Environment]::NewLine)", $encoding) +} + +function Format-TableCell { + <# + .SYNOPSIS + Renders comment text as a single-line markdown code span, truncated to + -MaxLength characters. + #> + param( + [Parameter(Mandatory = $true)] + [AllowEmptyString()] + [string]$Value, + [int]$MaxLength = 90 + ) + + $collapsed = ($Value -replace '\s+', ' ').Trim() + if ([string]::IsNullOrEmpty($collapsed)) { + return '' + } + if ($collapsed.Length -gt $MaxLength) { + $collapsed = $collapsed.Substring(0, $MaxLength).TrimEnd() + '...' + } + + # GitHub splits a table row on every unescaped pipe, inside a code span as much + # as outside one. A fence has to outnumber any backtick it wraps. + $escaped = $collapsed -replace '\|', '\|' + $fence = if ($escaped.Contains('`')) { '``' } else { '`' } + if ($escaped.StartsWith('`') -or $escaped.EndsWith('`')) { + $escaped = " $escaped " + } + + return "$fence$escaped$fence" +} + +function Format-FileLink { + param( + [Parameter(Mandatory = $true)] + [string]$Path, + [Parameter(Mandatory = $true)] + [int]$Line + ) + + $label = "${Path}:$Line" + if ([string]::IsNullOrWhiteSpace($Repository) -or [string]::IsNullOrWhiteSpace($Sha)) { + return "``$label``" + } + + return "[$label]($ServerUrl/$Repository/blob/$Sha/$Path#L$Line)" +} + +function Get-RunReference { + if ([string]::IsNullOrWhiteSpace($Repository) -or [string]::IsNullOrWhiteSpace($RunId)) { + return 'this check''s log' + } + + return "[this check's log]($ServerUrl/$Repository/actions/runs/$RunId)" +} + +if (-not (Test-Path -LiteralPath $ReportPath)) { + throw "Comment-hygiene report not found: $ReportPath" +} + +$report = Get-Content -LiteralPath $ReportPath -Raw | ConvertFrom-Json +foreach ($field in @('base', 'violationCount', 'violations')) { + if ($null -eq $report.PSObject.Properties[$field]) { + throw "Comment-hygiene report is missing the '$field' field: $ReportPath" + } +} + +$violationCount = [int]$report.violationCount +$baseLabel = [string]$report.base +$hasViolations = $violationCount -gt 0 + +if ($hasViolations) { + $listed = @(@($report.violations) | Select-Object -First $MaxListed) + $commentLines = @( + '### Comment hygiene (advisory)' + '' + "$violationCount comment-style violation(s) in the lines this branch adds since ``$baseLabel``." + 'Advisory only -- no check fails on these, and the same violations appear as inline warnings on the Files changed tab.' + '' + '| File | Category | Comment |' + '| --- | --- | --- |' + ) + + foreach ($violation in $listed) { + $commentLines += ('| {0} | `{1}` | {2} |' -f + (Format-FileLink -Path ([string]$violation.file) -Line ([int]$violation.line)), + ([string]$violation.category), + (Format-TableCell -Value ([string]$violation.text))) + } + + $commentLines += '' + if ($violationCount -gt $listed.Count) { + $commentLines += "$($violationCount - $listed.Count) more not listed here -- see $(Get-RunReference)." + $commentLines += '' + } + + $commentLines += "Fix them per ``$SkillPath``." + $commentLines += 'Running `.\build.ps1 -CommentHygiene` (or `.\test.ps1 -CommentHygiene`) enforces them locally, and' + $commentLines += 're-wraps over-wide lines and repairs non-ASCII punctuation as it goes.' +} +else { + $commentLines = @( + '### Comment hygiene (advisory)' + '' + "No comment-style violations in the lines this branch adds since ``$baseLabel``." + ) +} + +$commentDirectory = Split-Path -Path $CommentPath -Parent +if (-not [string]::IsNullOrWhiteSpace($commentDirectory) -and -not (Test-Path -LiteralPath $commentDirectory)) { + New-Item -ItemType Directory -Path $commentDirectory -Force | Out-Null +} + +$utf8NoBom = New-Object System.Text.UTF8Encoding($false) +[System.IO.File]::WriteAllText($CommentPath, ($commentLines -join [System.Environment]::NewLine), $utf8NoBom) + +$resolvedCommentPath = [System.IO.Path]::GetFullPath($CommentPath) +Write-GitHubOutputValue -Name 'comment_path' -Value $resolvedCommentPath +Write-GitHubOutputValue -Name 'has_violations' -Value ($hasViolations.ToString().ToLowerInvariant()) +Write-GitHubOutputValue -Name 'violation_count' -Value ([string]$violationCount) + +Write-Output ([pscustomobject]@{ + CommentPath = $resolvedCommentPath + HasViolations = $hasViolations + ViolationCount = $violationCount +}) diff --git a/Build/Agent/CommentHygiene.psm1 b/Build/Agent/CommentHygiene.psm1 index f06f3aeb2f..e9b502c85a 100644 --- a/Build/Agent/CommentHygiene.psm1 +++ b/Build/Agent/CommentHygiene.psm1 @@ -77,7 +77,7 @@ function Get-CommentHygieneEditorConfig { whatever .editorconfig already declares for every file, so the two can never drift apart. Only the [*] section is read, since that is where this repo declares both values. Results are cached per root -- this runs - once per gate invocation, not once per file. + once per scan, not once per file. .OUTPUTS A hashtable with MaxLineLength and TabWidth. Falls back to 98 and 4 when @@ -560,7 +560,7 @@ function Get-CommentLineClassification { else { # Inlined from Get-CommentBody: this loop runs every line of every diffed file on # every build, where a per-line function call plus its own redundant Trim measurably - # slows the gate down. + # slows the scan down. if ($trimmed.StartsWith('///')) { $kinds[$i] = 'exempt' $bodies[$i] = $trimmed.Substring(3) @@ -633,7 +633,7 @@ function Get-CommentHygieneViolations { $allowedLines = $LineFilter[$file] } - # File.ReadAllLines, not Get-Content: ~50x faster on a large file, and this gate runs + # File.ReadAllLines, not Get-Content: ~50x faster on a large file, and this scan runs # every build. Still BOM-safe: StreamReader sniffs a real BOM even given an explicit # encoding. $lines = [System.IO.File]::ReadAllLines($file, [System.Text.Encoding]::UTF8) diff --git a/Build/Agent/Test-BuildCommentHygieneComment.ps1 b/Build/Agent/Test-BuildCommentHygieneComment.ps1 new file mode 100644 index 0000000000..02aef3f8a8 --- /dev/null +++ b/Build/Agent/Test-BuildCommentHygieneComment.ps1 @@ -0,0 +1,136 @@ +<# +.SYNOPSIS + Smoke test for Build-CommentHygieneComment.ps1. Run directly under both + PowerShell 7 and Windows PowerShell 5.1. +#> +[CmdletBinding()] +param() + +Set-StrictMode -Version Latest +$ErrorActionPreference = 'Stop' + +$scriptPath = Join-Path $PSScriptRoot 'Build-CommentHygieneComment.ps1' +$workspace = Join-Path ([System.IO.Path]::GetTempPath()) ('BuildCommentHygieneCommentTest-' + [guid]::NewGuid().ToString('N')) + +function Assert-True { + param( + [Parameter(Mandatory = $true)] + [bool]$Condition, + [Parameter(Mandatory = $true)] + [string]$Message + ) + + if (-not $Condition) { + throw $Message + } +} + +function New-Report { + param( + [Parameter(Mandatory = $true)] + [string]$Path, + [Parameter(Mandatory = $true)] + [AllowEmptyCollection()] + [object[]]$Violations + ) + + $report = [ordered]@{ + base = 'origin/main' + advisory = $true + violationCount = $Violations.Count + violations = @($Violations) + } + + $directory = Split-Path -Path $Path -Parent + if (-not (Test-Path -LiteralPath $directory)) { + New-Item -ItemType Directory -Path $directory -Force | Out-Null + } + + $encoding = New-Object System.Text.UTF8Encoding($false) + [System.IO.File]::WriteAllText($Path, ($report | ConvertTo-Json -Depth 5), $encoding) +} + +try { + $violationsDirectory = Join-Path $workspace 'with-violations' + $cleanDirectory = Join-Path $workspace 'clean' + + # A pipe and a backtick in the flagged text: both would break the table row or + # its code span if the renderer passed them through. + $awkward = 'Chooses `Name` when the flag is set | otherwise the abbreviation' + $violations = @( + [ordered]@{ file = 'Src/xWorks/RecordEditView.cs'; line = 42; category = 'comment-too-long'; text = $awkward } + [ordered]@{ file = 'Build/Agent/Sample.ps1'; line = 7; category = 'doc-pointer'; text = 'See the design note' } + [ordered]@{ file = 'Src/xWorks/xWorks.csproj'; line = 3; category = 'xml-illegal-double-hyphen'; text = 'Keeps the pack step honest' } + ) + + $violationsReportPath = Join-Path $violationsDirectory 'report.json' + $violationsCommentPath = Join-Path $violationsDirectory 'comment.md' + $violationsOutputPath = Join-Path $violationsDirectory 'github-output.txt' + New-Report -Path $violationsReportPath -Violations $violations + + $violationsResult = & $scriptPath -ReportPath $violationsReportPath -CommentPath $violationsCommentPath ` + -GitHubOutputPath $violationsOutputPath -MaxListed 2 -Repository 'sillsdev/FieldWorks' ` + -RunId '123456' -ServerUrl 'https://github.com' -Sha '0123456789abcdef0123456789abcdef01234567' + + Assert-True (Test-Path -LiteralPath $violationsCommentPath) 'Expected the helper to write the comment markdown file.' + Assert-True ($violationsResult.HasViolations -eq $true) 'Expected the helper to report violations.' + Assert-True ($violationsResult.ViolationCount -eq 3) 'Expected the helper to report every violation in the count.' + + $comment = Get-Content -LiteralPath $violationsCommentPath -Raw + Assert-True ($comment.Contains('3 comment-style violation(s) in the lines this branch adds since `origin/main`.')) ` + 'Expected the comment to open with the violation count and the base ref.' + Assert-True ($comment.Contains('[Src/xWorks/RecordEditView.cs:42](https://github.com/sillsdev/FieldWorks/blob/0123456789abcdef0123456789abcdef01234567/Src/xWorks/RecordEditView.cs#L42)')) ` + 'Expected each row to link the file and line at the head commit.' + Assert-True ($comment.Contains('otherwise the abbreviation')) 'Expected the flagged comment text in the row.' + Assert-True (-not ($comment -match '(?m)^\| \[Src/xWorks/xWorks\.csproj')) 'Expected -MaxListed to cap the table rows.' + Assert-True ($comment.Contains('1 more not listed here -- see [this check''s log](https://github.com/sillsdev/FieldWorks/actions/runs/123456).')) ` + 'Expected the capped remainder to point at the run log.' + + # Every table row has to keep exactly the four pipes its three columns need, so + # a pipe inside the flagged text stays escaped rather than splitting the row. + foreach ($line in ($comment -split '\r?\n')) { + if (-not $line.StartsWith('| [')) { continue } + $barePipes = ([regex]::Matches($line, '(?$null + # Reads local origin/HEAD, not `git remote show origin`, which hits the network + # every run. --quiet keeps git's stderr out of PowerShell's error stream, where + # it terminates instead of falling through. + $originHead = git rev-parse --verify --quiet --abbrev-ref origin/HEAD if ($LASTEXITCODE -eq 0 -and -not [string]::IsNullOrWhiteSpace($originHead)) { return $originHead.Trim() } return 'origin/main' } @@ -162,36 +169,75 @@ function Write-Violation { } } +function Write-ScanReport { + <# + .SYNOPSIS + Writes the scan result to -ReportPath as JSON. Does nothing when the + caller asked for no report. + + .PARAMETER Base + The ref the scan diffed against, recorded verbatim in the report. + #> + param( + [object[]] $Violations, + [string] $Base + ) + + if ([string]::IsNullOrWhiteSpace($ReportPath)) { return } + + # PowerShell's current location and .NET's working directory can differ; New-Item + # reads the first and WriteAllText the second, so anchor a relative path once. + $path = $ReportPath + if (-not [System.IO.Path]::IsPathRooted($path)) { + $path = Join-Path (Get-Location).Path $path + } + + $directory = Split-Path -Path $path -Parent + if (-not [string]::IsNullOrWhiteSpace($directory) -and -not (Test-Path -LiteralPath $directory)) { + New-Item -ItemType Directory -Path $directory -Force | Out-Null + } + + $records = New-Object System.Collections.ArrayList + foreach ($v in $Violations) { + [void]$records.Add([ordered]@{ + file = ($v.File.Substring($repoRoot.Length + 1) -replace '\\', '/') + line = $v.Line + category = $v.Category + text = $v.Text + }) + } + + $report = [ordered]@{ + base = $Base + advisory = [bool]$Advisory + violationCount = $records.Count + violations = @($records.ToArray()) + } + + # UTF8Encoding($false): a byte-order mark ahead of the opening brace makes the file + # unparsable to a plain JSON reader. + [System.IO.File]::WriteAllText($path, ($report | ConvertTo-Json -Depth 5), + (New-Object System.Text.UTF8Encoding($false))) + Write-Host "comment-hygiene: wrote $($records.Count) violation record(s) to $path" +} + if ($Full) { $files = git ls-files $scopedGlobs | ForEach-Object { ConvertTo-RepoPath $_ } | Where-Object { -not (Test-ExcludedPath $_) } $violations = Get-CommentHygieneViolations -Files $files Write-Host "comment-hygiene -Full: $($violations.Count) violation(s) across $($files.Count) file(s)" foreach ($v in $violations) { Write-Violation $v } + Write-ScanReport -Violations $violations -Base 'HEAD' exit 0 } -# The build and test CI steps both invoke this gate over the same tree, which would -# annotate every violation twice. The first run marks the job so the rest skip. -if ($env:GITHUB_ACTIONS -eq 'true') { - if ($env:FW_COMMENT_HYGIENE_REPORTED -eq '1') { - Write-Host 'comment-hygiene: already reported earlier in this job.' - exit 0 - } - if (-not [string]::IsNullOrWhiteSpace($env:GITHUB_ENV)) { - # UTF8Encoding($false): appending a BOM mid-file would corrupt the - # environment file the runner parses when the step ends. - [System.IO.File]::AppendAllText($env:GITHUB_ENV, "FW_COMMENT_HYGIENE_REPORTED=1`n", - (New-Object System.Text.UTF8Encoding($false))) - } -} - $base = Resolve-BaseRef -Explicit $BaseRef Write-Host "comment-hygiene: scanning lines added since $base" $lineFilter = Get-AddedLineFilter -Base $base if ($lineFilter.Count -eq 0) { Write-Host 'comment-hygiene: no added lines in scope to check.' + Write-ScanReport -Violations @() -Base $base exit 0 } @@ -265,6 +311,7 @@ foreach ($group in ($pending | Group-Object File)) { } $violations = $remainingViolations.ToArray() +Write-ScanReport -Violations $violations -Base $base if ($fixedCount -gt 0 -or $wrappedCount -gt 0) { Write-Host ("comment-hygiene: auto-fixed {0} punctuation and re-wrapped {1} over-long comment line(s) in {2} file(s) (review and include in your commit)." -f $fixedCount, $wrappedCount, $fixedFiles.Keys.Count) -ForegroundColor Yellow diff --git a/Build/Agent/powershell-compat.ps1 b/Build/Agent/powershell-compat.ps1 index 97e51f1423..dac023085e 100644 --- a/Build/Agent/powershell-compat.ps1 +++ b/Build/Agent/powershell-compat.ps1 @@ -1,10 +1,11 @@ <# .SYNOPSIS - Static PowerShell-version-compatibility gate for the comment-hygiene tooling. + Static compatibility check for PowerShell that must run under both 5.1 and 7. .DESCRIPTION - Two independent static layers, neither of which requires more than one - PowerShell engine to actually be installed: + Applies to any script this repo ships to developers or CI. Two independent + static layers, neither of which requires more than one PowerShell engine to + actually be installed: 1. A dependency-free regex scan for known gotchas where 5.1 and 7 both parse the same text successfully but disagree on its meaning, so no @@ -47,14 +48,11 @@ $ErrorActionPreference = 'Stop' Import-Module (Join-Path $PSScriptRoot 'CommentHygiene.psm1') -Force $repoRoot = (Resolve-Path (Join-Path $PSScriptRoot '../..')).Path -$targetFiles = @( - 'Build/Agent/CommentHygiene.psm1', - 'Build/Agent/CommentHygiene.Tests.ps1', - 'Build/Agent/comment-hygiene.ps1', - 'Build/Agent/comment-hygiene-repair.ps1', - 'Build/Agent/comment-hygiene-blame.ps1', - 'Build/Agent/powershell-compat.ps1' -) | ForEach-Object { Join-Path $repoRoot $_ } | Where-Object { Test-Path -LiteralPath $_ } +# Every PowerShell file under Build/Agent, so a new script is covered the day it +# lands rather than when someone remembers to name it here. +$targetFiles = @(Get-ChildItem -LiteralPath $PSScriptRoot -Recurse -File | + Where-Object { $_.Extension -eq '.ps1' -or $_.Extension -eq '.psm1' } | + ForEach-Object { $_.FullName } | Sort-Object) $violations = New-Object System.Collections.ArrayList diff --git a/build.ps1 b/build.ps1 index 2907c3b50e..3ff6eaa985 100644 --- a/build.ps1 +++ b/build.ps1 @@ -119,9 +119,8 @@ Defaults to the FW_BUILD_STARTED_BY environment variable when set, otherwise 'unknown'. .PARAMETER CommentHygiene - Enforce the comment-hygiene gate, failing the build on any violation in the lines this - branch adds. Required of coding agents; a developer build leaves it off and never runs - the gate. CI reports violations as warning annotations either way. + Enforce the comment-hygiene check, failing the build on any violation in the lines this + branch adds. .PARAMETER SkipWorktreeLock Internal switch used when build.ps1 is invoked from test.ps1 while the parent test workflow @@ -210,13 +209,10 @@ if (-not $runningOnWindows) { exit 1 } -# Comment hygiene is opt-in: with -CommentHygiene it blocks the build, in CI it -# only annotates the pull request, and an ordinary developer build is silent. -$commentHygieneInCI = ($env:GITHUB_ACTIONS -eq 'true') -or ($env:CI -eq 'true') -if ($CommentHygiene -or $commentHygieneInCI) { +if ($CommentHygiene) { $commentHygienePath = Join-Path $PSScriptRoot "Build/Agent/comment-hygiene.ps1" - & $commentHygienePath -Advisory:(-not $CommentHygiene) - if ($CommentHygiene -and $LASTEXITCODE -ne 0) { + & $commentHygienePath + if ($LASTEXITCODE -ne 0) { exit $LASTEXITCODE } } diff --git a/test.ps1 b/test.ps1 index 28126417e0..10f99419ef 100644 --- a/test.ps1 +++ b/test.ps1 @@ -37,9 +37,8 @@ Equivalent environment variable: FW_TEST_ALLOW_ASSERT_DIALOGS=1. .PARAMETER CommentHygiene - Enforce the comment-hygiene gate, failing the run on any violation in the lines this - branch adds. Required of coding agents; a developer run leaves it off and never runs - the gate. + Enforce the comment-hygiene check, failing the run on any violation in the lines this + branch adds. .PARAMETER StartedBy Optional actor label written to worktree lock metadata (for example: user or agent). @@ -112,13 +111,10 @@ param( $ErrorActionPreference = 'Stop' -# Comment hygiene is opt-in: with -CommentHygiene it blocks the run, in CI it -# only annotates the pull request, and an ordinary developer run is silent. -$commentHygieneInCI = ($env:GITHUB_ACTIONS -eq 'true') -or ($env:CI -eq 'true') -if ($CommentHygiene -or $commentHygieneInCI) { +if ($CommentHygiene) { $commentHygienePath = Join-Path $PSScriptRoot "Build/Agent/comment-hygiene.ps1" - & $commentHygienePath -Advisory:(-not $CommentHygiene) - if ($CommentHygiene -and $LASTEXITCODE -ne 0) { + & $commentHygienePath + if ($LASTEXITCODE -ne 0) { exit $LASTEXITCODE } }