From 4aacb5f59fb49799e99bd8b156433ad7d47a394d Mon Sep 17 00:00:00 2001 From: Azure SDK Bot <53356347+azure-sdk@users.noreply.github.com> Date: Mon, 13 Sep 2021 13:04:47 -0700 Subject: [PATCH] Sync eng/common directory with azure-sdk-tools for PR 1946 (#2862) * Add ability to exit gracefully when all files in the diff are excluded * Address case where cspell exits with an error when all files from the 'files' config list are excluded by the expressions in 'ignorePaths' * Add tests * Review feedback: impl goes at the bottom and should be treated as a script, logic for testing should happen above that and exit appropriately if running tests * Import common instead of logging * Enable strict mode Co-authored-by: Daniel Jurek --- .../check-spelling-in-changed-files.ps1 | 412 +++++++++++++----- 1 file changed, 303 insertions(+), 109 deletions(-) diff --git a/eng/common/scripts/check-spelling-in-changed-files.ps1 b/eng/common/scripts/check-spelling-in-changed-files.ps1 index b0a961162..ebeddc605 100644 --- a/eng/common/scripts/check-spelling-in-changed-files.ps1 +++ b/eng/common/scripts/check-spelling-in-changed-files.ps1 @@ -1,123 +1,17 @@ -[CmdletBinding()] -Param ( - [Parameter()] - [string] $TargetBranch, - - [Parameter()] - [string] $SourceBranch, - - [Parameter()] - [string] $CspellConfigPath = "./.vscode/cspell.json", - - [Parameter()] - [switch] $ExitWithError -) - -$ErrorActionPreference = "Continue" -. $PSScriptRoot/logging.ps1 - -if ((Get-Command git | Measure-Object).Count -eq 0) { - LogError "Could not locate git. Install git https://git-scm.com/downloads" - exit 1 -} - -if ((Get-Command npx | Measure-Object).Count -eq 0) { - LogError "Could not locate npx. Install NodeJS (includes npx and npx) https://nodejs.org/en/download/" - exit 1 -} - -if (!(Test-Path $CspellConfigPath)) { - LogError "Could not locate config file $CspellConfigPath" - exit 1 -} - -# Lists names of files that were in some way changed between the -# current $SourceBranch and $TargetBranch. Excludes files that were deleted to -# prevent errors in Resolve-Path -Write-Host "git diff --diff-filter=d --name-only $TargetBranch $SourceBranch" -$changedFiles = git diff --diff-filter=d --name-only $TargetBranch $SourceBranch ` - | Resolve-Path - -$changedFilesCount = ($changedFiles | Measure-Object).Count -Write-Host "Git Detected $changedFilesCount changed file(s). Files checked by cspell may exclude files according to cspell.json" - -if ($changedFilesCount -eq 0) { - Write-Host "No changes detected" - exit 0 -} - -$changedFilePaths = @() -foreach ($file in $changedFiles) { - $changedFilePaths += $file.Path -} - -# Using GetTempPath because it works on linux and windows. Setting .json -# extension because cspell requires the file have a .json or .jsonc extension -$cspellConfigTemporaryPath = Join-Path ` - ([System.IO.Path]::GetTempPath()) ` - "$([System.IO.Path]::GetRandomFileName()).json" - -$cspellConfigContent = Get-Content $CspellConfigPath -Raw -$cspellConfig = ConvertFrom-Json $cspellConfigContent - -# If the config has no "files" property this adds it. If the config has a -# "files" property this sets the value, overwriting the existing value. In this -# case, spell checking is only intended to check files from $changedFiles so -# preexisting entries in "files" will be overwritten. -Add-Member ` - -MemberType NoteProperty ` - -InputObject $cspellConfig ` - -Name "files" ` - -Value $changedFilePaths ` - -Force - -# Set the temporary config file with the mutated configuration. The temporary -# location is used to configure the command and the original file remains -# unchanged. -Write-Host "Setting config in: $cspellConfigTemporaryPath" -Set-Content ` - -Path $cspellConfigTemporaryPath ` - -Value (ConvertTo-Json $cspellConfig -Depth 100) - -# Use the mutated configuration file when calling cspell -Write-Host "npx cspell lint --config $cspellConfigTemporaryPath" -$spellingErrors = npx cspell lint --config $cspellConfigTemporaryPath - -if ($spellingErrors) { - $errorLoggingFunction = Get-Item 'Function:LogWarning' - if ($ExitWithError) { - $errorLoggingFunction = Get-Item 'Function:LogError' - } - - foreach ($spellingError in $spellingErrors) { - &$errorLoggingFunction $spellingError - } - &$errorLoggingFunction "Spelling errors detected. To correct false positives or learn about spell checking see: https://aka.ms/azsdk/engsys/spellcheck" - - if ($ExitWithError) { - exit 1 - } -} else { - Write-Host "No spelling errors detected. Removing temporary config file." - Remove-Item -Path $cspellConfigTemporaryPath -Force -} - -exit 0 - <# .SYNOPSIS Uses cspell (from NPM) to check spelling of recently changed files .DESCRIPTION -This script checks files that have changed relative to a base branch (default -branch) for spelling errors. Dictionaries and spelling configurations reside +This script checks files that have changed relative to a base branch (default +branch) for spelling errors. Dictionaries and spelling configurations reside in a configurable `cspell.json` location. This script uses `npx` and assumes that NodeJS (and by extension `npm` and `npx`) are installed on the machine. If it does not detect `npx` it will warn the user and exit with an error. -The entire file is scanned, not just changed sections. Spelling errors in parts +The entire file is scanned, not just changed sections. Spelling errors in parts of the file not touched will still be shown. This script copies the config file supplied in CspellConfigPath to a temporary @@ -144,6 +38,9 @@ Optional location to use for cspell.json path. Default value is .PARAMETER ExitWithError Exit with error code 1 if spelling errors are detected. +.PARAMETER Test +Run test functions against the script logic + .EXAMPLE ./eng/common/scripts/check-spelling-in-changed-files.ps1 -TargetBranch 'target_branch_name' @@ -151,3 +48,300 @@ This will run spell check with changes in the current branch with respect to `target_branch_name` #> + +[CmdletBinding()] +Param ( + [Parameter()] + [string] $TargetBranch, + + [Parameter()] + [string] $SourceBranch, + + [Parameter()] + [string] $CspellConfigPath = "./.vscode/cspell.json", + + [Parameter()] + [switch] $ExitWithError, + + [Parameter()] + [switch] $Test +) + +Set-StrictMode -Version 3.0 + +function TestSpellChecker() { + Test-Exit0WhenAllFilesExcluded + ResetTest + Test-Exit1WhenIncludedFileHasSpellingError + ResetTest + Test-Exit0WhenIncludedFileHasNoSpellingError + ResetTest + Test-Exit1WhenChangedFileAlreadyHasSpellingError + ResetTest + Test-Exit0WhenUnalteredFileHasSpellingError + ResetTest + Test-Exit0WhenSpellingErrorsAndNoExitWithError +} + +function Test-Exit0WhenAllFilesExcluded() { + # Arrange + "sepleing errrrrorrrrr" > ./excluded/excluded-file.txt + git add -A + git commit -m "One change" + + # Act + &"$PSScriptRoot/check-spelling-in-changed-files.ps1" ` + -TargetBranch HEAD~1 ` + -ExitWithError + + # Assert + if ($LASTEXITCODE -ne 0) { + throw "`$LASTEXITCODE != 0" + } +} + +function Test-Exit1WhenIncludedFileHasSpellingError() { + # Arrange + "sepleing errrrrorrrrr" > ./included/included-file.txt + git add -A + git commit -m "One change" + + # Act + &"$PSScriptRoot/check-spelling-in-changed-files.ps1" ` + -TargetBranch HEAD~1 ` + -ExitWithError + + # Assert + if ($LASTEXITCODE -ne 1) { + throw "`$LASTEXITCODE != 1" + } +} + +function Test-Exit0WhenIncludedFileHasNoSpellingError() { + # Arrange + "correct spelling" > ./included/included-file.txt + git add -A + git commit -m "One change" + + # Act + &"$PSScriptRoot/check-spelling-in-changed-files.ps1" ` + -TargetBranch HEAD~1 ` + -ExitWithError + + # Assert + if ($LASTEXITCODE -ne 0) { + throw "`$LASTEXITCODE != 0" + } +} + +function Test-Exit1WhenChangedFileAlreadyHasSpellingError() { + # Arrange + "sepleing errrrrorrrrr" > ./included/included-file.txt + git add -A + git commit -m "First change" + + "A statement without spelling errors" >> ./included/included-file.txt + git add -A + git commit -m "Second change" + + # Act + &"$PSScriptRoot/check-spelling-in-changed-files.ps1" ` + -TargetBranch HEAD~1 ` + -ExitWithError + + # Assert + if ($LASTEXITCODE -ne 1) { + throw "`$LASTEXITCODE != 1" + } +} + +function Test-Exit0WhenUnalteredFileHasSpellingError() { + # Arrange + "sepleing errrrrorrrrr" > ./included/included-file-1.txt + git add -A + git commit -m "One change" + + "A statement without spelling errors" > ./included/included-file-2.txt + git add -A + git commit -m "Second change" + + # Act + &"$PSScriptRoot/check-spelling-in-changed-files.ps1" ` + -TargetBranch HEAD~1 ` + -ExitWithError + + # Assert + if ($LASTEXITCODE -ne 0) { + throw "`$LASTEXITCODE != 0" + } +} + +function Test-Exit0WhenSpellingErrorsAndNoExitWithError() { + # Arrange + "sepleing errrrrorrrrr" > ./included/included-file-1.txt + git add -A + git commit -m "One change" + + # Act + &"$PSScriptRoot/check-spelling-in-changed-files.ps1" ` + -TargetBranch HEAD~1 + + # Assert + if ($LASTEXITCODE -ne 0) { + throw "`$LASTEXITCODE != 0" + } +} + +function SetupTest($workingDirectory) { + Write-Host "Create test temp dir: $workingDirectory" + New-Item -ItemType Directory -Force -Path $workingDirectory | Out-Null + + Push-Location $workingDirectory | Out-Null + git init + + New-Item -ItemType Directory -Force -Path "./excluded" + New-Item -ItemType Directory -Force -Path "./included" + New-Item -ItemType Directory -Force -Path "./.vscode" + + "Placeholder" > "./excluded/placeholder.txt" + "Placeholder" > "./included/placeholder.txt" + + $configJsonContent = @" +{ + "version": "0.1", + "language": "en", + "ignorePaths": [ + ".vscode/cspell.json", + "excluded/**" + ] +} +"@ + $configJsonContent > "./.vscode/cspell.json" + + git add -A + git commit -m "Init" +} + +function ResetTest() { + # Empty out the working tree + git checkout . + git clean -xdf + + $revCount = git rev-list --count HEAD + if ($revCount -gt 1) { + # Reset N-1 changes so there is only the initial commit + $revisionsToReset = $revCount - 1 + git reset --hard HEAD~$revisionsToReset + } +} + +function TeardownTest($workingDirectory) { + Pop-Location | Out-Null + Write-Host "Remove test temp dir: $workingDirectory" + Remove-Item -Path $workingDirectory -Recurse -Force | Out-Null +} + +if ($Test) { + $workingDirectory = Join-Path ([System.IO.Path]::GetTempPath()) ([System.IO.Path]::GetRandomFileName()) + + SetupTest $workingDirectory + TestSpellChecker + TeardownTest $workingDirectory + Write-Host "Test complete" + exit 0 +} + +$ErrorActionPreference = "Continue" +. $PSScriptRoot/common.ps1 + +if ((Get-Command git | Measure-Object).Count -eq 0) { + LogError "Could not locate git. Install git https://git-scm.com/downloads" + exit 1 +} + +if ((Get-Command npx | Measure-Object).Count -eq 0) { + LogError "Could not locate npx. Install NodeJS (includes npx and npx) https://nodejs.org/en/download/" + exit 1 +} + +if (!(Test-Path $CspellConfigPath)) { + LogError "Could not locate config file $CspellConfigPath" + exit 1 +} + +# Lists names of files that were in some way changed between the +# current $SourceBranch and $TargetBranch. Excludes files that were deleted to +# prevent errors in Resolve-Path +Write-Host "git diff --diff-filter=d --name-only $TargetBranch $SourceBranch" +$changedFiles = git diff --diff-filter=d --name-only $TargetBranch $SourceBranch ` + | Resolve-Path + +$changedFilesCount = ($changedFiles | Measure-Object).Count +Write-Host "Git Detected $changedFilesCount changed file(s). Files checked by cspell may exclude files according to cspell.json" + +if ($changedFilesCount -eq 0) { + Write-Host "No changes detected" + exit 0 +} + +$changedFilePaths = @() +foreach ($file in $changedFiles) { + $changedFilePaths += $file.Path +} + +# The "files" list must always contain a file which exists, is not empty, and is +# not excluded in ignorePaths. In this case it will be a file with the contents +# "1" (no spelling errors will be detected) +$notExcludedFile = (New-TemporaryFile).ToString() +"1" >> $notExcludedFile +$changedFilePaths += $notExcludedFile + +$cspellConfigContent = Get-Content $CspellConfigPath -Raw +$cspellConfig = ConvertFrom-Json $cspellConfigContent + +# If the config has no "files" property this adds it. If the config has a +# "files" property this sets the value, overwriting the existing value. In this +# case, spell checking is only intended to check files from $changedFiles so +# preexisting entries in "files" will be overwritten. +Add-Member ` + -MemberType NoteProperty ` + -InputObject $cspellConfig ` + -Name "files" ` + -Value $changedFilePaths ` + -Force + +# Set the temporary config file with the mutated configuration. The temporary +# location is used to configure the command and the original file remains +# unchanged. +Write-Host "Setting config in: $CspellConfigPath" +Set-Content ` + -Path $CspellConfigPath ` + -Value (ConvertTo-Json $cspellConfig -Depth 100) + +# Use the mutated configuration file when calling cspell +Write-Host "npx cspell lint --config $CspellConfigPath --no-must-find-files " +$spellingErrors = npx cspell lint --config $CspellConfigPath --no-must-find-files + +Write-Host "cspell run complete, restoring original configuration." +Set-Content -Path $CspellConfigPath -Value $cspellConfigContent -NoNewLine + +if ($spellingErrors) { + $errorLoggingFunction = Get-Item 'Function:LogWarning' + if ($ExitWithError) { + $errorLoggingFunction = Get-Item 'Function:LogError' + } + + foreach ($spellingError in $spellingErrors) { + &$errorLoggingFunction $spellingError + } + &$errorLoggingFunction "Spelling errors detected. To correct false positives or learn about spell checking see: https://aka.ms/azsdk/engsys/spellcheck" + + if ($ExitWithError) { + exit 1 + } +} else { + Write-Host "No spelling errors detected. Removing temporary file." + Remove-Item -Path $notExcludedFile -Force | Out-Null +} + +exit 0