[RULE] PSScriptAnalyzerSettingsSchema

[travisclagrone]

User Story

• As a writer of PSScriptAnalyzer settings files
• I want static code checking of the structure of PSScriptAnalyzer settings files
• so that I can catch schematic violations early and easily.

Specification Details

The static schema for an Invoke-ScriptAnalyzer -Settings hashtable is not formally documented in a rigorous format at present. However, the current implementation implicitly adheres to the following as expressed using JSON Schema (with the exception of case-sensitivity):

{
  "$schema": "http://json-schema.org/draft-07/schema",
  "title": "PSScriptAnalyzerSettings",
  "type": "object",
  "definitions": {
    "SeverityString": {
      "type": "string",
      "pattern": "Error|Warning|Information"
    },
    "stringOrStrings": {
      "oneOf": [
        {
          "type": "string"
        },
        {
          "type": "array",
          "items": {
            "type": "string"
          }
        }
      ]
    }
  },
  "properties": {
    "CustomRulePath": {
      "$ref": "#/definitions/stringOrStrings"
    },
    "ExcludeRules": {
      "$ref": "#/definitions/stringOrStrings"
    },
    "IncludeDefaultRules": {
      "type": "boolean"
    },
    "IncludeRules": {
      "$ref": "#/definitions/stringOrStrings"
    },
    "RecurseCustomRulePath": {
      "type": "boolean"
    },
    "Rules": {
      "type": "object",
      "additionalProperties": {
        "type": "object"
      }
    },
    "Severity": {
      "oneOf": [
        {
          "$ref": "#/definitions/SeverityString"
        },
        {
          "type": "array",
          "items": {
            "$ref": "#/definitions/SeverityString"
          }
        }
      ]
    }
  },
  "additionalProperties": false
}

The static schema of an Invoke-ScriptAnalyzer -Settings hashtable, as currently implemented and as expressed above (but with the addition of global case-insensitivity), shall serve as the specification for the proposed "PSScriptAnalyzerSettingsSchema" rule. Additional requirements are as follows:

• The schema utilized by the "PSScriptAnalyzerSettingsSchema" rule MUST be enforced by the Invoke-ScriptAnalyzer command.
• The implementation of the "PSScriptAnalyzerSettingsSchema" rule SHOULD share source code for validation logic with the Invoke-ScriptAnalyzer command in an appropriately modular fashion such that changes to the aforementioned schema's specification correspond one-to-one to changes in the source code.

PSScriptAnalyzer Version

The latest version of PSScriptAnalyzer as of this writing is 1.18.1.

[travisclagrone]

@bergmeister What do you recommend I should add, remove, and/or change in my proposed specification for the "PSScriptAnalyzerSettingSchema" rule? Thank you in advance!

[JamesWTruher]

@travis-c-lagrone Do you see this as a part of the invoke-scriptanalyzer experience, or would you see this as stand-alone operation, something along the lines of Test-ScriptAnalyzerSetting or both?

[travisclagrone]

@JamesWTruher Well, in terms of validating the -Settings parameter of Invoke-ScriptAnalyzer in a more structured manner, then "yes" it should definitely be part of Invoke-ScriptAnalyzer. But more to your point, I think it would be most beneficial if it were incorporated into the processing performed on the -Path/-ScriptDefinition parameter of Invoke-ScriptAnalyzer, so that the settings analysis could be accessed at development-time via integrated editor code-checking and intellisense. That being said, I do not think that full incorporation is achievable without extensive extension/refactoring, considering the difficulty of inferring whether a given input script to Invoke-ScriptAnalyzer is an instance of a PSScriptAnalyzer settings file.

Possible Architectures

The way I see it, there are three alternative ways of incorporating settings schema validation into PSScriptAnalyzer (listed in ascending order of both complexity and value):

1. A private method for Invoke-ScriptAnalyzer
2. A public command, Test-ScriptAnalyzerSettings
3. A public rule, PSScriptAnalyzerSettingsSchema

1. A private method for Invoke-ScriptAnalyzer

A private method for Invoke-ScriptAnalyzer would enable the implicit validation of its -Settings parameter at run-time. However, this architecture would not support the validation of arbitrary settings files without having to invoke the full script analyzer (e.g. at test-time in a CI/CD scenario). Nor would it support script analysis itself (e.g. PowerShellEditorServices' usage of PSScriptAnalyzer to provide interactive code checking, intellisense, and correction suggestions at development-time).

This architecture is very nearly implemented already as a switch statement case in the C# private void Settings.ParseSettingsHashtable(HashTable) method. Implementing this architecture would entail refactoring said extant logic into a dedicated method (which would additionally aid in unit testing), and might also include separating the validation logic from the actual parsing logic. (I will have to rely on others more expert than I as to the ideal architecture of a validating parser.)

2. A public command, Test-ScriptAnalyzerSettings

A public command, Test-ScriptAnalyzerSettings, would enable the explicit validation of arbitrary settings files at build-/test-time. Such a command could also be reused by Invoke-ScriptAnalyzer for the implicit input validation of its -Settings parameter at run-time (see no.1). However, this architecture would not support script analysis itself (e.g. PowerShellEditorServices' usage of PSScriptAnalyzer to provide interactive code checking, intellisense, and correction suggestions at development-time), at least not without extension/refactoring in each tool that incorporates Invoke-ScriptAnalyzer to also incorporate Test-ScriptAnalyzerSettings.

Implementing this architecture would entail creating a new cmdlet (which is a relatively straight-forward--albeit verbose--task), and possibly refactoring Invoke-ScriptAnalyzer to use it as well. The primary challenge of this architecture is how to either unify schema validation and parsing logic such that both Invoke-ScriptAnalyzer -Settings and Test-ScriptAnalyzerSettings depend upon a single source of truth, or (alternatively) enforce parity between Invoke-ScriptAnalyzerSettings and Test-ScriptAnalyzerSettings for the sake of maintaining consistency during any future code changes to one or other. (I will have to rely on others more expert than I as to the ideal architecture details in either case, but it seems apparent that the first option would be preferable, assuming it is feasible.)

3. A public rule, PSScriptAnalyzerSettingsSchema

A public rule, PSScriptAnalyzerSettingsSchema, would enable script analysis itself and thus the automatic and interactive validation of a settings file at development-time (e.g. PowerShellEditorServices' provision of code checking, intellisense, and correction suggestions). Such a rule could also be reused by a Test-ScriptAnalyzerSettings command to support the explicit validation of settings files at build-/test-time (see no.2), as well as by Invoke-ScriptAnalyzer for the implicit input validation of its -Settings parameter at run-time (see no.1).

This architecture would entail creating a new rule (which is a relatively straight-forward task, but would include significant work querying and reporting on the AST), and possibly reusing the rule to additionally implement both architectures no.2 and no.1, in which case additional work to translate DiagnosticRecords to exceptions would be required. The primary challenge of this architecture is how to infer whether a given script to be analyzed is intended to be a PSScriptAnalyzer settings files, and thus whether to apply the PSScriptAnalyzerSettingsSchema rule to any arbitrary input script of Invoke-ScriptAnalyzer.

Unfortunately, inferring whether a given script is supposed to be a PSScriptAnalyzer settings instance is a non-trivial challenge. Since the purpose of a PSScriptAnalyzerSettingsSchema rule is to enable automatic analysis, we can neither rely on simple invocation as an indication of applicability (as in the case of a Test-ScriptAnalyzerSettings command), nor can we rely on some sort of explicit (i.e. non-automatic) input when Invoke-ScriptAnalyzer is invoked (such as by adding a -IsSettingsScript flag parameter to Invoke-ScriptAnalyzer).

Possible Solutions

The way I see it, there are three alternative ways of how a PSScriptAnalyzerSettingsSchema rule could determine whether it applies to any given script, i.e. whether the script to be analyzed is intended to be a PSScriptAnalyzer settings instance (listed in ascending order of both complexity and value):

1. Infer from the script's filename
2. Identify with a custom source code attribute
3. Enable the rule with a new EnablePSScriptAnalyzerRule attribute

1. Infer from the script's filename

One possible solution is to infer rule applicability by way of the script's filename. Either the built-in standard file name ("PSScriptAnalyzerSettings.psd1") or any one of an optional set of user-configurable filenames/paths would be trigger the rule. (To clarify, the user-configured filenames would be passed via the -Settings argument of the invocation of Invoke-ScriptAnalyzer over the--separate--settings script file to be analyzed.) However, this would limit the rule to situations where Invoke-ScriptAnalyzer is invoked with the -Path parameter set rather than the -ScriptDefinition parameter set. Unfortunately, this limitation is likely prohibitive considering that the current implementation of PowerShellEditorServices (the single largest client of PSScriptAnalyzer) reduces all invocations of Invoke-ScriptAnalyzer to the -ScriptDefinition parameter set, to say nothing of the additionally limited reusability of a PSScriptAnalyzerSettingsSchema rule for Test-ScriptAnalyzerSettings and Invoke-ScriptAnalyzer -Settings.

2. Identify with a custom source code attribute

A second possible solution is to identify rule applicability by way of a custom attribute (e.g. PSScriptAnalyzerSettings) that is explicitly added to the setting's source code at level of the root hashtable. This solution would obviate the Invoke-ScriptAnalyzer -ScriptDefinition limitation of the filename-based approach (see no.1). However, it would not fix the limited reusability of a PSScriptAnalyzerSettingsSchema rule for Test-ScriptAnalyzerSettings and Invoke-ScriptAnalyzer -Settings, which should each consider their input to always be a PSScriptAnalyzer settings instance, regardless of whether or not it is annotated with a custom attribute identifying it as such.

3. Enable the rule with a new EnablePSScriptAnalyzerRule attribute

A third possible solution is to disable the PSScriptAnalyzerSettingsSchema by default, and then enable it when appropriate by way of a new EnablePSScriptAnalyzerRule attribute that is explicitly added to the setting's source code. This solution is superficially similar to solution no.2 in that it requires an explicit source code attribute and would obviate the Invoke-ScriptAnalyzer -ScriptDefinition limitation of solution no.1. However, it is more powerful in that it would also enable reusability of a PSScriptAnalyzerSettingsSchema rule for Test-ScriptAnalyzerSettings and Invoke-ScriptAnalyzer -Settings (i.e. the rule will execute in full if invoked, regardless of whether the script to be analyzed is annotated with an attribute this rule recognizes, whether the script's file has a name this rule recognizes, or even whether the script has a filename). Moreover, it would be relatively straightforward to also incorporate the filename-trigger approach by special-casing the rule within Invoke-ScriptAnalyzer, which would be justified considering that the PSScriptAnalyzerSettingSchema rule would be a built-in intrinsic to PSScriptAnalyzer. Finally, there would be various additional side benefits to having an EnablePSScriptAnalyzerRule setting and the ability to disable rules by default.

The drawbacks of this solution come not from any limitations in usefulness or reusability, but rather the additional work of first implementing a whole other feature: the EnablePSScriptAnalyzerRule.

Feedback

Considering the wide range of complexity and value in the alternatives discussed here, I would like to request feedback from all of @JamesWTruher, @bergmeister, @rjmholt, and @rkeithhill before determining how to proceed.