Repository navigation
[RULE] PSScriptAnalyzerSettingsSchema #1279
Description
Activity
Christoph Bergmeister (@bergmeister) What do you recommend I should add, remove, and/or change in my proposed specification for the "PSScriptAnalyzerSettingSchema" rule? Thank you in advance!
travis-c-lagrone Do you see this as a part of the
invoke-scriptanalyzerexperience, or would you see this as stand-alone operation, something along the lines ofTest-ScriptAnalyzerSettingor both?James Truher (@JamesWTruher) Well, in terms of validating the
-Settingsparameter ofInvoke-ScriptAnalyzerin a more structured manner, then "yes" it should definitely be part ofInvoke-ScriptAnalyzer. But more to your point, I think it would be most beneficial if it were incorporated into the processing performed on the-Path/-ScriptDefinitionparameter ofInvoke-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 toInvoke-ScriptAnalyzeris 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):
- A private method for
Invoke-ScriptAnalyzer - A public command,
Test-ScriptAnalyzerSettings - A public rule,
PSScriptAnalyzerSettingsSchema
1. A private method for
Invoke-ScriptAnalyzerA private method for
Invoke-ScriptAnalyzerwould enable the implicit validation of its-Settingsparameter 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
switchstatement 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-ScriptAnalyzerSettingsA public command,
Test-ScriptAnalyzerSettings, would enable the explicit validation of arbitrary settings files at build-/test-time. Such a command could also be reused byInvoke-ScriptAnalyzerfor the implicit input validation of its-Settingsparameter 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 incorporatesInvoke-ScriptAnalyzerto also incorporateTest-ScriptAnalyzerSettings.Implementing this architecture would entail creating a new cmdlet (which is a relatively straight-forward--albeit verbose--task), and possibly refactoring
Invoke-ScriptAnalyzerto use it as well. The primary challenge of this architecture is how to either unify schema validation and parsing logic such that bothInvoke-ScriptAnalyzer -SettingsandTest-ScriptAnalyzerSettingsdepend upon a single source of truth, or (alternatively) enforce parity betweenInvoke-ScriptAnalyzerSettingsandTest-ScriptAnalyzerSettingsfor 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,
PSScriptAnalyzerSettingsSchemaA 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 aTest-ScriptAnalyzerSettingscommand to support the explicit validation of settings files at build-/test-time (see no.2), as well as byInvoke-ScriptAnalyzerfor the implicit input validation of its-Settingsparameter 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 thePSScriptAnalyzerSettingsSchemarule to any arbitrary input script ofInvoke-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
PSScriptAnalyzerSettingsSchemarule is to enable automatic analysis, we can neither rely on simple invocation as an indication of applicability (as in the case of aTest-ScriptAnalyzerSettingscommand), nor can we rely on some sort of explicit (i.e. non-automatic) input whenInvoke-ScriptAnalyzeris invoked (such as by adding a-IsSettingsScriptflag parameter toInvoke-ScriptAnalyzer).Possible Solutions
The way I see it, there are three alternative ways of how a
PSScriptAnalyzerSettingsSchemarule 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):- Infer from the script's filename
- Identify with a custom source code attribute
- Enable the rule with a new
EnablePSScriptAnalyzerRuleattribute
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
-Settingsargument of the invocation ofInvoke-ScriptAnalyzerover the--separate--settings script file to be analyzed.) However, this would limit the rule to situations whereInvoke-ScriptAnalyzeris invoked with the-Pathparameter set rather than the-ScriptDefinitionparameter set. Unfortunately, this limitation is likely prohibitive considering that the current implementation of PowerShellEditorServices (the single largest client of PSScriptAnalyzer) reduces all invocations ofInvoke-ScriptAnalyzerto the-ScriptDefinitionparameter set, to say nothing of the additionally limited reusability of aPSScriptAnalyzerSettingsSchemarule forTest-ScriptAnalyzerSettingsandInvoke-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 theInvoke-ScriptAnalyzer -ScriptDefinitionlimitation of the filename-based approach (see no.1). However, it would not fix the limited reusability of aPSScriptAnalyzerSettingsSchemarule forTest-ScriptAnalyzerSettingsandInvoke-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
EnablePSScriptAnalyzerRuleattributeA third possible solution is to disable the
PSScriptAnalyzerSettingsSchemaby default, and then enable it when appropriate by way of a newEnablePSScriptAnalyzerRuleattribute 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 theInvoke-ScriptAnalyzer -ScriptDefinitionlimitation of solution no.1. However, it is more powerful in that it would also enable reusability of aPSScriptAnalyzerSettingsSchemarule forTest-ScriptAnalyzerSettingsandInvoke-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 withinInvoke-ScriptAnalyzer, which would be justified considering that thePSScriptAnalyzerSettingSchemarule would be a built-in intrinsic to PSScriptAnalyzer. Finally, there would be various additional side benefits to having anEnablePSScriptAnalyzerRulesetting 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 James Truher (@JamesWTruher), Christoph Bergmeister (@bergmeister), Rob Holt (@rjmholt), and Keith Hill (@rkeithhill) before determining how to proceed.
- A private method for
User Story
Specification Details
The static schema for an
Invoke-ScriptAnalyzer -Settingshashtable 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 -Settingshashtable, 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:Invoke-ScriptAnalyzercommand.Invoke-ScriptAnalyzercommand 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.