Skip to content

Improve settings format, allow JSON + custom settings formats #1552

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
rjmholt opened this issue Jul 22, 2020 · 3 comments
Open

Improve settings format, allow JSON + custom settings formats #1552

rjmholt opened this issue Jul 22, 2020 · 3 comments

Comments

@rjmholt
Copy link
Contributor

rjmholt commented Jul 22, 2020

Currently PSScriptAnalyzer settings suffer from the following drawbacks:

  • They are opaque and truly documented only by the parsing logic
  • Misconfiguration usually fails silently, or at best issues an unhelpful generic error
  • They must be specified in PSD format, forcing a PSD parser to be present
  • Configurable rules cannot easily specify arbitrary configurations to be read from settings
  • Setting properties on configurable rules can conflict with constructors (they are set after constructor and default value evaluation)
  • Settings (and Invoke-ScriptAnalyzer parameters) have ambiguous duplication like Rules, IncludeRules and ExcludeRules
  • The default Enable rule configuration is false...
  • Rule names vs namespaces aren't delineated, so it can't be known from settings where the namespace vs name of a rule begins/ends

Instead, PSScriptAnalyzer settings should:

  • Be self-documenting when possible
  • Issue useful errors about where and why settings parsing failed
  • Be specifiable in multiple convenient ways that don't require PowerShell (particularly in JSON), and allow extension to specify them in more ways
  • Allow arbitrary structured configuration for rules, to be defined by those rules, and read configurations in as objects for the rule to consume
  • Pass configurations to rules in the constructor call, to allow rules to apply the configurations at the correct time and with their own logic
  • Make settings simple and self-documenting, with no ambiguous concepts
  • Enable rules by default, while offering a way to disable them while preserving their configuration entry
  • Specify rules by name and namespace, separating them with a / character

As such, the proposed new settings will provide:

  • An extensible API to provide settings
  • A better way to provide settings in PSD format (and the same from a hashtable object)
  • A new way to provide settings as JSON

PSSA API structure

PSSA will provide an extensible API to provide configurations. This consists of a top level Script Analyzer configuration:

public interface IScriptAnalyzerConfiguration
{
BuiltinRulePreference? BuiltinRules { get; }
RuleExecutionMode? RuleExecution { get; }
IReadOnlyList<string> RulePaths { get; }
IReadOnlyDictionary<string, IRuleConfiguration> RuleConfiguration { get; }
}

An individual rule configuration:

public class CommonConfiguration
{
public static CommonConfiguration Default = new CommonConfiguration(enabled: true);
[JsonConstructor]
public CommonConfiguration(bool enabled)
{
Enabled = enabled;
}
public bool Enabled { get; } = true;
}
public interface IRuleConfiguration
{
CommonConfiguration Common { get; }
IRuleConfiguration AsTypedConfiguration(Type configurationType);
}
public class RuleConfiguration : IRuleConfiguration
{
public static RuleConfiguration Default { get; } = new RuleConfiguration(CommonConfiguration.Default);
public RuleConfiguration(CommonConfiguration common)
{
Common = common;
}
public CommonConfiguration Common { get; }
public virtual IRuleConfiguration AsTypedConfiguration(Type configurationType)
{
if (configurationType == typeof(RuleConfiguration))
{
return this;
}
return null;
}
}

And the ability to implement a subclass/implementation of IRuleConfiguration and inject it into a rule's constructor by specifying a rule as generic:

public abstract class ScriptRule<TConfiguration> : Rule<TConfiguration> where TConfiguration : IRuleConfiguration
{
protected ScriptRule(RuleInfo ruleInfo, TConfiguration ruleConfiguration) : base(ruleInfo, ruleConfiguration)
{
}
public abstract IReadOnlyList<ScriptDiagnostic> AnalyzeScript(Ast ast, IReadOnlyList<Token> tokens, string scriptPath);
}

Script Analyzer is able to use this configuration type in the generic parameter to deserialise the rule configuration when it is contructed.

PSD settings

Implementing these APIs for PSD format, a typical PSD settings file will look like this:

@{
    BuiltinRules = "Default" # Or "None" | "Aggressive" (name changeable)
    RuleExecution = "Default" # Or "Sequential" | "Parallel", possibly others depending on executor implementations
    RulePaths = @(
        "C:\somewhere\rules.dll"
        "path/relative/to/host/configured/root/module.psm1"
        "/also/allow/directory/module/"
    )
    RuleConfiguration = @(
        @{ Rule = "PS/AvoidAliases"; Configuration = @{ Enable = $false } }
        @{
            Rule = "PS/UseCompatibleCommands"
            Configuration = @{
                TargetPlatforms = @(
                    @{ OS = "Linux" }
                    @{ OS = "MacOS" }
                )
            }
        }
       @{
            Rule = "PS/AnotherRule"
            Configuration = @{
                ModeEnum = "CustomMode"
                Numbers = @(1, 2, 3)
            }
        }
        @{ Rule = "PS/RuleWithoutConfiguration" }
        @{ Rule = "ExternalRules/ExternallyImplementedRule" }
    )
}

Setting fields that aren't provided will have sensible defaults, and explicit default options allowing for the same. When rule settings are deserialised for the first time, failures will be reported (I haven't gotten to this part yet, but I would like to make the settings tell users where the failure occurred, what was given and what was expected).

Hashtable settings

Settings provided by Hashtable object will follow exactly the same conventions as those given above for PSD format, except converting from an in-memory hashtable object. A PSD file and its hashtable result in PowerShell will convert to the same settings.

JSON settings

Similar to the PSD settings, the JSON settings will essentially offer a new syntax for rule settings:

{
    "BuiltinRules": "Default",
    "RuleExecution": "Default",
    "RulePaths": [
        "C:\\somewhere\\rules.dll",
        "path/relative/to/host/configured/root/module.psm1",
        "/also/allow/directory/module/",
    ],
    "RuleConfiguration": [
        { "Rule": "PS/AvoidAliases", "Configuration": { "Enable": false } },
        {
            "Rule": "PS/UseCompatibleCommands",
            "Configuration": {
                "TargetPlatforms": [
                    { "OS": "Linux" },
                    { "OS": "MacOS" }
                ]
            }
        },
        {
            "Rule": "PS/AnotherRule",
            "Configuration": {
                "ModeEnum": "CustomMode",
                "Numbers": [1, 2, 3]
            }
        },
        { "Rule": "PS/RuleWithoutConfiguration" },
        { "Rule": "ExternalRules/ExternallyImplementedRule" }
    ]
}

Deserialisation to objects

Settings in PSD and JSON form will be deserialised to the type requested by the rule they configure. For JSON, this will use Newtonsoft.Json deserialisation, so allowing all attributes it supports. In the case of PSD, those attributes will also be supported. For example:

var obj = _converter.Convert<DataContractObject>("@{ FirstName = 'Raymond'; FamilyName = 'Yacht' }");
Assert.Equal("Raymond", obj.FirstName);
Assert.Equal("Yacht", obj.LastName);
Assert.Null(obj.MiddleName);

@MartinSGill
Copy link

Love it. I'd also really like it if the formatter had a default settings file you could place in project root, e.g. .psformatter or something similar and Invoke-Formatter looks for that file is not settings are provided.

@alexchandel
Copy link

I just want to disable a rule 😭😭😭

@luckman212
Copy link

@alexchandel Me too. Did you figure out how to do it? I've been at it for over an hour...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants