Sync eng/common directory with azure-sdk-tools for PR 2352 (#3145)

* Add spell check pin changes

* Apply suggestions from code review

Co-authored-by: Wes Haggard <weshaggard@users.noreply.github.com>

* Add package-lock.json

* Set node version and upgrade npm in check-spelling.yml

* Do not mutate local NPM environment

* Remove unnecessary comment, we no longer use npm exec

Co-authored-by: Daniel Jurek <djurek@microsoft.com>
Co-authored-by: Wes Haggard <weshaggard@users.noreply.github.com>

eng/common/pipelines/templates/steps/check-spelling.yml

@@ -1,6 +1,6 @@
 # Checks spelling of files that changed between the current state of the repo
 # and some ref (branch, tag, etc.) or commit hash. Only runs on PRs.
-# ContinueOnError   - true: Pipeline warns on spelling error 
+# ContinueOnError   - true: Pipeline warns on spelling error
 #                     false: Pipeline fails on spelling error
 # TargetBranch      - Target ref (e.g. main) to compare to create file change
 #                     list.
@@ -14,13 +14,18 @@ parameters:
 
 steps:
   - ${{ if eq(variables['Build.Reason'], 'PullRequest') }}:
+    - task: NodeTool@0
+      inputs:
+        versionSpec: 16.x
+      displayName: Use Node.js 16.x
+
     - task: PowerShell@2
       displayName: Check spelling (cspell)
       continueOnError: ${{ parameters.ContinueOnError }}
-      inputs: 
+      inputs:
         targetType: filePath
         filePath: eng/common/scripts/check-spelling-in-changed-files.ps1
-        arguments: >- 
+        arguments: >-
           -TargetBranch "origin/$("${{ parameters.TargetBranch }}" -replace "refs/heads/")"
           -SourceBranch ${{ parameters.SourceBranch }}
           -CspellConfigPath ${{ parameters.CspellConfigPath }}

eng/common/scripts/check-spelling-in-changed-files.ps1

@@ -29,7 +29,11 @@ Git ref to use instead of changes in current repo state. Use `HEAD` here to
 check spelling of files that have been committed and exclude any new files or
 modified files that are not committed. This is most useful in CI scenarios where
 builds may have modified the state of the repo. Leaving this parameter blank  
-includes files for whom changes have not been committed. 
+includes files for whom changes have not been committed.
+
+.PARAMETER SpellCheckRoot
+Root folder from which to generate relative paths for spell checking. Mostly
+used in testing.
 
 .PARAMETER CspellConfigPath
 Optional location to use for cspell.json path. Default value is 
@@ -58,7 +62,10 @@ Param (
     [string] $SourceBranch,
 
     [Parameter()]
-    [string] $CspellConfigPath = "./.vscode/cspell.json",
+    [string] $CspellConfigPath = (Resolve-Path "$PSScriptRoot/../../../.vscode/cspell.json"),
+
+    [Parameter()]
+    [string] $SpellCheckRoot = (Resolve-Path "$PSScriptRoot/../../../"),
 
     [Parameter()]
     [switch] $ExitWithError,
@@ -92,6 +99,8 @@ function Test-Exit0WhenAllFilesExcluded() {
     # Act
     &"$PSScriptRoot/check-spelling-in-changed-files.ps1" `
         -TargetBranch HEAD~1 `
+        -CspellConfigPath "./.vscode/cspell.json" `
+        -SpellCheckRoot "./" `
         -ExitWithError
 
     # Assert
@@ -109,6 +118,8 @@ function Test-Exit1WhenIncludedFileHasSpellingError() {
     # Act
     &"$PSScriptRoot/check-spelling-in-changed-files.ps1" `
         -TargetBranch HEAD~1 `
+        -CspellConfigPath "./.vscode/cspell.json" `
+        -SpellCheckRoot "./" `
         -ExitWithError
 
     # Assert
@@ -126,6 +137,8 @@ function Test-Exit0WhenIncludedFileHasNoSpellingError() {
     # Act
     &"$PSScriptRoot/check-spelling-in-changed-files.ps1" `
         -TargetBranch HEAD~1 `
+        -CspellConfigPath "./.vscode/cspell.json" `
+        -SpellCheckRoot "./" `
         -ExitWithError
 
     # Assert
@@ -147,6 +160,8 @@ function Test-Exit1WhenChangedFileAlreadyHasSpellingError() {
     # Act
     &"$PSScriptRoot/check-spelling-in-changed-files.ps1" `
         -TargetBranch HEAD~1 `
+        -CspellConfigPath "./.vscode/cspell.json" `
+        -SpellCheckRoot "./" `
         -ExitWithError
 
     # Assert
@@ -168,6 +183,8 @@ function Test-Exit0WhenUnalteredFileHasSpellingError() {
     # Act
     &"$PSScriptRoot/check-spelling-in-changed-files.ps1" `
         -TargetBranch HEAD~1 `
+        -CspellConfigPath "./.vscode/cspell.json" `
+        -SpellCheckRoot "./" `
         -ExitWithError
 
     # Assert
@@ -184,7 +201,9 @@ function Test-Exit0WhenSpellingErrorsAndNoExitWithError() {
 
     # Act
     &"$PSScriptRoot/check-spelling-in-changed-files.ps1" `
-        -TargetBranch HEAD~1
+        -TargetBranch HEAD~1 `
+        -CspellConfigPath "./.vscode/cspell.json" `
+        -SpellCheckRoot "./"
 
     # Assert
     if ($LASTEXITCODE -ne 0) {
@@ -259,11 +278,6 @@ if ((Get-Command git | Measure-Object).Count -eq 0) {
     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
@@ -272,9 +286,18 @@ if (!(Test-Path $CspellConfigPath)) {
 # 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
+Write-Host "git diff --diff-filter=d --name-only --relative $TargetBranch $SourceBranch"
+$changedFilesList = git diff `
+  --diff-filter=d `
+  --name-only `
+  --relative `
+  $TargetBranch `
+  $SourceBranch
+
+$changedFiles = @()
+foreach ($file in $changedFilesList) {
+  $changedFiles += Resolve-Path $file
+}
 
 $changedFilesCount = ($changedFiles | Measure-Object).Count
 Write-Host "Git Detected $changedFilesCount changed file(s). Files checked by cspell may exclude files according to cspell.json"
@@ -289,41 +312,10 @@ 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
+$spellingErrors = &"$PSScriptRoot/../spelling/Invoke-Cspell.ps1" `
+  -CspellConfigPath $CspellConfigPath `
+  -SpellCheckRoot $SpellCheckRoot `
+  -ScanGlobs $changedFilePaths
 
 if ($spellingErrors) {
     $errorLoggingFunction = Get-Item 'Function:LogWarning'
@@ -340,8 +332,7 @@ if ($spellingErrors) {
         exit 1
     }
 } else {
-    Write-Host "No spelling errors detected. Removing temporary file."
-    Remove-Item -Path $notExcludedFile -Force | Out-Null
+  Write-Host "No spelling errors detected"
 }
 
 exit 0

eng/common/spelling/Invoke-Cspell.ps1

@@ -0,0 +1,194 @@
+<#
+.SYNOPSIS
+Invokes cspell using dependencies defined in adjacent ./package*.json
+
+.PARAMETER JobType
+Maps to cspell command (e.g. `lint`, `trace`, etc.). Default is `lint`
+
+.PARAMETER ScanGlobs
+List of glob expressions to be scanned. This list is not constrained by
+npx/cmd's upper limit on command line length as the globs are inserted into the
+cspell config's `files` property.
+
+.PARAMETER CSpellConfigPath
+Location of cspell.json file to use when scanning. Defaults to
+`.vscode/cspell.json` at the root of the repo.
+
+.PARAMETER SpellCheckRoot
+Location of root folder for generating readable relative file paths. Defaults to
+the root of the repo relative to the script.
+
+.PARAMETER PackageInstallCache
+Location of a working directory. If no location is provided a folder will be
+created in the temp folder, package*.json files will be placed in that folder.
+
+.PARAMETER LeavePackageInstallCache
+If set the PackageInstallCache will not be deleted. Use if there are multiple
+calls to Invoke-Cspell.ps1 to prevent creating multiple working directories and
+redundant calls `npm install`.
+
+.PARAMETER Test
+Run test functions against the script logic
+
+.EXAMPLE
+./eng/common/scripts/Invoke-Cspell.ps1 -ScanGlobs 'sdk/*/*/PublicAPI/**/*.md'
+
+This will run spell check with the given globs
+
+.EXAMPLE
+./eng/common/scripts/Invoke-Cspell.ps1 -ScanGlobs @('sdk/storage/**', 'sdk/keyvault/**')
+
+This will run spell check against multiple globs
+
+.EXAMPLE
+./eng/common/scripts/Invoke-Cspell.ps1 -ScanGlobs './README.md'
+
+This will run spell check against a single file
+
+#>
+[CmdletBinding()]
+param(
+  [Parameter()]
+  [string] $JobType = 'lint',
+
+  [Parameter()]
+  [array]$ScanGlobs = '**',
+
+  [Parameter()]
+  [string] $CSpellConfigPath = (Resolve-Path "$PSScriptRoot/../../../.vscode/cspell.json"),
+
+  [Parameter()]
+  [string] $SpellCheckRoot = (Resolve-Path "$PSScriptRoot/../../.."),
+
+  [Parameter()]
+  [string] $PackageInstallCache = (Join-Path ([System.IO.Path]::GetTempPath()) "cspell-tool-path"),
+
+  [Parameter()]
+  [switch] $LeavePackageInstallCache,
+
+  [Parameter()]
+  [switch] $Test
+)
+
+Set-StrictMode -Version 3.0
+
+if (!(Get-Command npm -ErrorAction SilentlyContinue)) {
+  LogError "Could not locate npm. Install NodeJS (includes npm) https://nodejs.org/en/download/"
+  exit 1
+}
+
+if (!(Test-Path $CSpellConfigPath)) {
+  LogError "Could not locate config file $CSpellConfigPath"
+  exit 1
+}
+
+function Test-VersionReportMatches() {
+  # Arrange
+  $expectedPackageVersion = '5.12.3'
+
+  # Act
+  $actual = &"$PSSCriptRoot/Invoke-Cspell.ps1" `
+    -JobType '--version'
+
+  # Assert
+  if ($actual -ne $expectedPackageVersion) {
+    throw "Mismatched version. Expected:`n$expectedPackageVersion`n`nActual:`n$actual"
+  }
+}
+
+function TestInvokeCspell() {
+  Test-VersionReportMatches
+}
+
+if ($Test) {
+  TestInvokeCspell
+  Write-Host "Test complete"
+  exit 0
+}
+
+# Prepare the working directory if it does not already have requirements in
+# place.
+if (!(Test-Path $PackageInstallCache)) {
+  New-Item -ItemType Directory -Path $PackageInstallCache | Out-Null
+}
+
+if (!(Test-Path "$PackageInstallCache/package.json")) {
+  Copy-Item "$PSScriptRoot/package.json" $PackageInstallCache
+}
+
+if (!(Test-Path "$PackageInstallCache/package-lock.json")) {
+  Copy-Item "$PSScriptRoot/package-lock.json" $PackageInstallCache
+}
+
+$deleteNotExcludedFile = $false
+$notExcludedFile = ""
+if (Test-Path "$SpellCheckRoot/LICENSE") {
+  $notExcludedFile = "$SpellCheckRoot/LICENSE"
+} elseif (Test-Path "$SpellCheckRoot/LICENSE.txt") {
+  $notExcludedFile = "$SpellCheckRoot/LICENSE.txt"
+} else {
+  # If there is no LICENSE file, fall back to creating a temporary file
+  # 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 = Join-Path $SpellCheckRoot ([System.IO.Path]::GetRandomFileName())
+  "1" >> $notExcludedFile
+  $deleteNotExcludedFile = $true
+}
+$ScanGlobs += $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 $ScanGlobs so
+# preexisting entries in "files" will be overwritten.
+Add-Member `
+  -MemberType NoteProperty `
+  -InputObject $cspellConfig `
+  -Name "files" `
+  -Value $ScanGlobs `
+  -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)
+
+# Before changing the run location, resolve paths specified in parameters
+$CSpellConfigPath = Resolve-Path $CSpellConfigPath
+$SpellCheckRoot = Resolve-Path $SpellCheckRoot
+
+$originalLocation = Get-Location
+
+try {
+  Set-Location $PackageInstallCache
+  npm install | Out-Null
+
+  # Use the mutated configuration file when calling cspell
+  $command = "npx --no-install cspell $JobType --config $CSpellConfigPath --no-must-find-files --root $SpellCheckRoot --relative"
+  Write-Host $command
+  $cspellOutput = npx  `
+    --no-install `
+    cspell `
+    $JobType `
+    --config $CSpellConfigPath `
+    --no-must-find-files `
+    --root $SpellCheckRoot `
+    --relative
+} finally {
+  Set-Location $originalLocation
+
+  Write-Host "cspell run complete, restoring original configuration and removing temp file."
+  Set-Content -Path $CSpellConfigPath -Value $cspellConfigContent -NoNewLine
+
+  if ($deleteNotExcludedFile) {
+    Remove-Item -Path $notExcludedFile
+  }
+}
+
+return $cspellOutput

eng/common/spelling/package.json

@@ -0,0 +1,10 @@
+{
+  "name": "cspell-version-pin",
+  "version": "0.1.1",
+  "dependencies": {
+    "@cspell/cspell-bundled-dicts": "^5.12.3",
+    "@cspell/cspell-types": "^5.12.3",
+    "cspell": "^5.12.3",
+    "cspell-lib": "^5.12.3"
+  }
+}