Skip to content

Commit

Permalink
Sync eng/common directory with azure-sdk-tools for PR 2352 (#25615)
Browse files Browse the repository at this point in the history
* Add spell check pin changes

* Apply suggestions from code review

Co-authored-by: Wes Haggard <[email protected]>

* 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 <[email protected]>
Co-authored-by: Wes Haggard <[email protected]>
  • Loading branch information
3 people authored Dec 2, 2021
1 parent b407041 commit 8ba684a
Show file tree
Hide file tree
Showing 5 changed files with 2,420 additions and 51 deletions.
11 changes: 8 additions & 3 deletions eng/common/pipelines/templates/steps/check-spelling.yml
Original file line number Diff line number Diff line change
@@ -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.
Expand All @@ -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 }}
Expand Down
87 changes: 39 additions & 48 deletions eng/common/scripts/check-spelling-in-changed-files.ps1
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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) {
Expand Down Expand Up @@ -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
Expand All @@ -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"
Expand All @@ -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'
Expand All @@ -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
194 changes: 194 additions & 0 deletions eng/common/spelling/Invoke-Cspell.ps1
Original file line number Diff line number Diff line change
@@ -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
Loading

0 comments on commit 8ba684a

Please sign in to comment.