PSUseConsistentIndentation: Check indentation of lines where first token is a LParen not followed by comment or new line

[liamjpeters]

PR Summary

Currently if an LParen ( is the first token on a line and the next token is not a comment or new line, then the line's indentation is not checked.

This is due to this if-check:

PSScriptAnalyzer/Rules/UseConsistentIndentation.cs

Lines 165 to 177 in a754b95

	case TokenKind.LParen:
	// When a line starts with a parenthesis and it is not the last non-comment token of that line,
	// then indentation does not need to be increased.
	if ((tokenIndex == 0 || tokens[tokenIndex - 1].Kind == TokenKind.NewLine) &&
	NextTokenIgnoringComments(tokens, tokenIndex)?.Kind != TokenKind.NewLine)
	{
	onNewLine = false;
	lParenSkippedIndentation.Push(true);
	break;
	}
	lParenSkippedIndentation.Push(false);
	AddViolation(token, indentationLevel++, diagnosticRecords, ref onNewLine);
	break;

AddViolation(), which subsequently checks the lines indentation against the expected indentation, is not called if the conditional evaluates to true.

This PR changes the logic to always call AddViolation(), so the indentation is checked, but to only increase the indentation level when the conditional evaluates to false.

Fixes #1994

I've run this rule recursively over some large code-bases, including the PSScriptAnalyzer repo.

• In the current 1.22.0 we find 2,927 violations.
• With this PR applied, we find 2,931 violations.
• I believe the settings I used (below) means the rule defaults to spaces and many of the files were using tabs - hence the large number of violations.

  $Settings = @{
      IncludeRules = @('PSUseConsistentIndentation')
      Rules        = @{
          PSUseConsistentIndentation = @{
              Enable = $true
          }
      }
  }

• All previously found violations were still found.
• There were 4 newly found violations:

  • \Tests\Engine\ModuleHelp.Tests.ps1\ModuleHelp.Tests.ps1 line 48
    

    PSScriptAnalyzer/Tests/Engine/ModuleHelp.Tests.ps1

    Lines 47 to 54 in a754b95

    	if ((-not ((($commandModuleName = $CommandInfo.Module.Name) -and ($commandVersion = $CommandInfo.Module.Version)) -or
    	(($commandModuleName = $CommandInfo.PSSnapin) -and ($commandVersion = $CommandInfo.PSSnapin.Version))))) {
    	Write-Error "For $($CommandInfo.Name) : Can't find PSSnapin/module name and version"
    	}
    	else {
    	# "For $commandName : Module is $commandModuleName. Version is $commandVersion"
    	[PSCustomObject]@{ CommandName = $CommandInfo.Name; ModuleName = $commandModuleName; Version = $commandVersion }
    	}

    This becomes:

    

    Which is a little dubious 🤔

    Edit: This is because the previous line has 6 opening LParens, and 3 closing RParens. So the indentation level is 3 greater than the previous line

  • \Tests\Engine\ModuleHelp.Tests.ps1\ModuleHelp.Tests.ps1 line 116
    

    PSScriptAnalyzer/Tests/Engine/ModuleHelp.Tests.ps1

    Lines 115 to 117 in a754b95

    	It "gets example code from <CommandName>" -TestCases $testCases {
    	($Help.Examples.Example | Select-Object -First 1).Code | Should -Not -BeNullOrEmpty
    	}

    This line wasn't previously being checked. It's using tabs, and as mentioned above the settings is defaulting to spaces.

  • \Tests\Engine\CommunityAnalyzerRules\CommunityAnalyzerRules.psm1\CommunityAnalyzerRules.psm1 line 518
    

    PSScriptAnalyzer/Tests/Engine/CommunityAnalyzerRules/CommunityAnalyzerRules.psm1

    Lines 515 to 521 in a754b95

    	{
    	# Checks nesting structures
    	$nestingASTs = $asts | Where-Object {($PSItem.Extent.StartLineNumber -gt $ast.Extent.StartLineNumber) -and
    	($PSItem.Extent.EndLineNumber -lt $ast.Extent.EndLineNumber)}
    	# If one AST have end-of-line comments, we should skip it.
    	[bool]$needComment = $ast.Extent.EndScriptPosition.Line.Trim().EndsWith("}")

    This is changed to:

    

  • \Tests\Rules\UseToExportFieldsInManifest.tests.ps1\UseToExportFieldsInManifest.tests.ps1 line 73

    This is a line that randomly uses tabs

    

PR Checklist

• PR has a meaningful title
  • Use the present tense and imperative mood when describing your changes
• Summarized changes
• Change is not breaking
• Make sure all .cs, .ps1 and .psm1 files have the correct copyright header
• Make sure you've added a new test if existing tests do not effectively test the code changed and/or updated documentation
• This PR is ready to merge and is not Work in Progress.
  • If the PR is work in progress, please add the prefix WIP: to the beginning of the title and remove the prefix when the PR is ready.