From 6ec93e315fcb58fdb07471195c9cb18f690a8805 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 bc59b554ec70b5818870bbf4cd749fcece6b11dc 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 dd9b2c22bf703069dc36b0b733e2c44d9c251dce 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 1c2ce3ec4311cc830593bc2e86edb09bef860aa4 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 1a43fd6dc9a6b91011da29477232969029423d15 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 be67df707dd9ec5e7df7fd0365400bc3e2203fa0 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