From 32b3ee40b846e28dbd4ea94c329ee1faca404bce Mon Sep 17 00:00:00 2001 From: Daniel Jurek Date: Fri, 27 Aug 2021 13:46:59 -0700 Subject: [PATCH 1/6] Add ability to exit gracefully when all files in the diff are excluded --- eng/common/scripts/check-spelling-in-changed-files.ps1 | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/eng/common/scripts/check-spelling-in-changed-files.ps1 b/eng/common/scripts/check-spelling-in-changed-files.ps1 index b0a9611620d5..8e75abd64256 100644 --- a/eng/common/scripts/check-spelling-in-changed-files.ps1 +++ b/eng/common/scripts/check-spelling-in-changed-files.ps1 @@ -79,9 +79,14 @@ Set-Content ` -Path $cspellConfigTemporaryPath ` -Value (ConvertTo-Json $cspellConfig -Depth 100) +# IGNORE_FILE_ -- In some cases a PR contains changes to only files which are +# excluded. In these cases `cspell` will produce an error when the files listed +# in the "files" config are excluded. Specifying a file name on the command line +# (even one which does not exist) will prevent this error. + # Use the mutated configuration file when calling cspell -Write-Host "npx cspell lint --config $cspellConfigTemporaryPath" -$spellingErrors = npx cspell lint --config $cspellConfigTemporaryPath +Write-Host "npx cspell lint --config $cspellConfigTemporaryPath --no-must-find-files IGNORE_FILE_" +$spellingErrors = npx cspell lint --config $cspellConfigTemporaryPath --no-must-find-files IGNORE_FILE_ if ($spellingErrors) { $errorLoggingFunction = Get-Item 'Function:LogWarning' From 110bc1b3d7e53df5de3f19b15853f4ff0cc17b0f Mon Sep 17 00:00:00 2001 From: Daniel Jurek Date: Fri, 27 Aug 2021 15:35:03 -0700 Subject: [PATCH 2/6] Address case where cspell exits with an error when all files from the 'files' config list are excluded by the expressions in 'ignorePaths' --- .../check-spelling-in-changed-files.ps1 | 30 ++++++++++++++----- 1 file changed, 22 insertions(+), 8 deletions(-) diff --git a/eng/common/scripts/check-spelling-in-changed-files.ps1 b/eng/common/scripts/check-spelling-in-changed-files.ps1 index 8e75abd64256..d118b2389fcf 100644 --- a/eng/common/scripts/check-spelling-in-changed-files.ps1 +++ b/eng/common/scripts/check-spelling-in-changed-files.ps1 @@ -51,6 +51,13 @@ 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 + # 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 ` @@ -79,14 +86,20 @@ Set-Content ` -Path $cspellConfigTemporaryPath ` -Value (ConvertTo-Json $cspellConfig -Depth 100) -# IGNORE_FILE_ -- In some cases a PR contains changes to only files which are -# excluded. In these cases `cspell` will produce an error when the files listed -# in the "files" config are excluded. Specifying a file name on the command line -# (even one which does not exist) will prevent this error. +# cspell will merge configurations from files in well-known locations. In this +# case the `.vscode/cspell.json` file is a well-known location. To force cspell +# to use ONLY the temporary config file we rename the config file in the +# well-known location to a temporary name. (`.vscode/cspell.json.ignore`) +Write-Host "Renaming $CspellConfigPath" +$originalConfigPath = Resolve-Path $CspellConfigPath +Move-Item $originalConfigPath "$originalConfigPath.ignore" | Out-Null # Use the mutated configuration file when calling cspell -Write-Host "npx cspell lint --config $cspellConfigTemporaryPath --no-must-find-files IGNORE_FILE_" -$spellingErrors = npx cspell lint --config $cspellConfigTemporaryPath --no-must-find-files IGNORE_FILE_ +Write-Host "npx cspell lint --config $cspellConfigTemporaryPath --no-must-find-files " +$spellingErrors = npx cspell lint --config $cspellConfigTemporaryPath --no-must-find-files + +Write-Host "cspell run complete, restoring original configuration." +Move-Item "$originalConfigPath.ignore" $originalConfigPath -Force | Out-Null if ($spellingErrors) { $errorLoggingFunction = Get-Item 'Function:LogWarning' @@ -94,7 +107,7 @@ if ($spellingErrors) { $errorLoggingFunction = Get-Item 'Function:LogError' } - foreach ($spellingError in $spellingErrors) { + 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" @@ -104,7 +117,8 @@ if ($spellingErrors) { } } else { Write-Host "No spelling errors detected. Removing temporary config file." - Remove-Item -Path $cspellConfigTemporaryPath -Force + Remove-Item -Path $cspellConfigTemporaryPath -Force | Out-Null + Remove-Item -Path $notExcludedFile -Force | Out-Null } exit 0 From 07d236e6e327953cb70b91d081bb64299c975d08 Mon Sep 17 00:00:00 2001 From: Daniel Jurek Date: Fri, 10 Sep 2021 11:49:47 -0700 Subject: [PATCH 3/6] Add tests --- .../check-spelling-in-changed-files.ps1 | 429 +++++++++++++----- 1 file changed, 303 insertions(+), 126 deletions(-) diff --git a/eng/common/scripts/check-spelling-in-changed-files.ps1 b/eng/common/scripts/check-spelling-in-changed-files.ps1 index d118b2389fcf..647e8d73f667 100644 --- a/eng/common/scripts/check-spelling-in-changed-files.ps1 +++ b/eng/common/scripts/check-spelling-in-changed-files.ps1 @@ -1,127 +1,3 @@ -[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 -} - -# 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 - -# 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) - -# cspell will merge configurations from files in well-known locations. In this -# case the `.vscode/cspell.json` file is a well-known location. To force cspell -# to use ONLY the temporary config file we rename the config file in the -# well-known location to a temporary name. (`.vscode/cspell.json.ignore`) -Write-Host "Renaming $CspellConfigPath" -$originalConfigPath = Resolve-Path $CspellConfigPath -Move-Item $originalConfigPath "$originalConfigPath.ignore" | Out-Null - -# Use the mutated configuration file when calling cspell -Write-Host "npx cspell lint --config $cspellConfigTemporaryPath --no-must-find-files " -$spellingErrors = npx cspell lint --config $cspellConfigTemporaryPath --no-must-find-files - -Write-Host "cspell run complete, restoring original configuration." -Move-Item "$originalConfigPath.ignore" $originalConfigPath -Force | Out-Null - -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 | Out-Null - Remove-Item -Path $notExcludedFile -Force | Out-Null -} - -exit 0 <# .SYNOPSIS @@ -129,14 +5,14 @@ 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 +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 @@ -163,6 +39,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' @@ -170,3 +49,301 @@ 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 +) + +function CheckSpellingInChangedFilesImpl() { + $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 + } + + # 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 + } +} + +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) { + CheckSpellingInChangedFilesImpl +} else { + $workingDirectory = Join-Path ([System.IO.Path]::GetTempPath()) ([System.IO.Path]::GetRandomFileName()) + + SetupTest $workingDirectory + TestSpellChecker + TeardownTest $workingDirectory +} + +exit 0 From 48b48e5484a4d8cd1ced084240ec14581da45807 Mon Sep 17 00:00:00 2001 From: Daniel Jurek Date: Mon, 13 Sep 2021 10:43:11 -0700 Subject: [PATCH 4/6] 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 --- .../check-spelling-in-changed-files.ps1 | 199 +++++++++--------- 1 file changed, 98 insertions(+), 101 deletions(-) diff --git a/eng/common/scripts/check-spelling-in-changed-files.ps1 b/eng/common/scripts/check-spelling-in-changed-files.ps1 index 647e8d73f667..db11ff5a2366 100644 --- a/eng/common/scripts/check-spelling-in-changed-files.ps1 +++ b/eng/common/scripts/check-spelling-in-changed-files.ps1 @@ -4,7 +4,7 @@ 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 +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. @@ -67,102 +67,6 @@ Param ( [Parameter()] [switch] $Test ) - -function CheckSpellingInChangedFilesImpl() { - $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 - } - - # 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 - } -} - function TestSpellChecker() { Test-Exit0WhenAllFilesExcluded ResetTest @@ -286,7 +190,6 @@ function Test-Exit0WhenSpellingErrorsAndNoExitWithError() { } } - function SetupTest($workingDirectory) { Write-Host "Create test temp dir: $workingDirectory" New-Item -ItemType Directory -Force -Path $workingDirectory | Out-Null @@ -336,14 +239,108 @@ function TeardownTest($workingDirectory) { Remove-Item -Path $workingDirectory -Recurse -Force | Out-Null } -if (!$Test) { - CheckSpellingInChangedFilesImpl -} else { +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/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 +} + +# 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 From d08fb64a82a4336efd8973274b425ab02119b1d7 Mon Sep 17 00:00:00 2001 From: Daniel Jurek Date: Mon, 13 Sep 2021 10:46:58 -0700 Subject: [PATCH 5/6] Import common instead of logging --- eng/common/scripts/check-spelling-in-changed-files.ps1 | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/eng/common/scripts/check-spelling-in-changed-files.ps1 b/eng/common/scripts/check-spelling-in-changed-files.ps1 index db11ff5a2366..d4581df84844 100644 --- a/eng/common/scripts/check-spelling-in-changed-files.ps1 +++ b/eng/common/scripts/check-spelling-in-changed-files.ps1 @@ -251,7 +251,7 @@ if ($Test) { $ErrorActionPreference = "Continue" -. $PSScriptRoot/logging.ps1 +. $PSScriptRoot/common.ps1 if ((Get-Command git | Measure-Object).Count -eq 0) { LogError "Could not locate git. Install git https://git-scm.com/downloads" From 6511c414dfc7f2dabdf901ffd6750423f00e2272 Mon Sep 17 00:00:00 2001 From: Daniel Jurek Date: Mon, 13 Sep 2021 10:50:32 -0700 Subject: [PATCH 6/6] Enable strict mode --- eng/common/scripts/check-spelling-in-changed-files.ps1 | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/eng/common/scripts/check-spelling-in-changed-files.ps1 b/eng/common/scripts/check-spelling-in-changed-files.ps1 index d4581df84844..ebeddc605032 100644 --- a/eng/common/scripts/check-spelling-in-changed-files.ps1 +++ b/eng/common/scripts/check-spelling-in-changed-files.ps1 @@ -1,4 +1,3 @@ - <# .SYNOPSIS Uses cspell (from NPM) to check spelling of recently changed files @@ -67,6 +66,9 @@ Param ( [Parameter()] [switch] $Test ) + +Set-StrictMode -Version 3.0 + function TestSpellChecker() { Test-Exit0WhenAllFilesExcluded ResetTest @@ -182,7 +184,7 @@ function Test-Exit0WhenSpellingErrorsAndNoExitWithError() { # Act &"$PSScriptRoot/check-spelling-in-changed-files.ps1" ` - -TargetBranch HEAD~1 ` + -TargetBranch HEAD~1 # Assert if ($LASTEXITCODE -ne 0) { @@ -249,7 +251,6 @@ if ($Test) { exit 0 } - $ErrorActionPreference = "Continue" . $PSScriptRoot/common.ps1