-
Notifications
You must be signed in to change notification settings - Fork 409
WIP: Add support for JSON Settings Files #2134
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
base: main
Are you sure you want to change the base?
Conversation
…sets. They are still copied to the built module in the Settings folder. The settings refactor will more naturally sit in the Engine\Settings directory. Note that we only move the Settings Presets - the Command Data Files will move elsewhere
Previously they were mixed in with the Settings Preset files. This wasn't an issue as all settings file were always PSD1 and these command data files are JSON. If we are to support other settings file formats, then we need to separate these. They are now copied to a CommandDataFiles directory in the built module. Two rules load these and will need to be fixed up to load them from the new directory: - Rules/AvoidOverwritingBuiltInCmdlets.cs - Rules/UseCompatibleCmdlets.cs
- Added a new `HashtableSettingsConverter` to convert inline PowerShell hashtables into a strongly typed `SettingsData`. - Introduced `ISettingsParser` interface for different settings file formats. - Implemented `Psd1SettingsParser` for parsing PowerShell data files (.psd1) into `SettingsData`. - Created a new `Settings` class to centralize the logic for obtaining analyzer settings, supporting auto-discovery, presets, and inline hashtables. - Removed the old `Settings` class and its associated logic - Updated `Helper.cs` and `ScriptAnalyzer.cs` to accommodate changes in settings handling. - Added `SettingsData` class to represent fully parsed and normalized settings.
…tory in AvoidOverwritingBuiltInCmdlets and UseCompatibleCmdlets rules
…ject for instantiation
'Parser' has one-way connotations. Settle on 'Serialize' over 'Serialise' (as the rest of the project is in American English and mine and copilots mixed usage was getting confusing). Stream -> string in the Deserialize interface method as it wasn't helpful like I thought it might be
…le to include rule options (called options to not cause confusion with settings)
|
@liamjpeters Will there still be back-compatibility for existing setting files, which I think will be a must? |
|
👋 @bergmeister - I hadn't put much thought into JSON's edge cases - especially with duplicate keys - so that's really valuable feedback - thank you 🙂. This is WIP and I will hopefully go back to it at some point - unless this is something the project feel strongly isn't right for PSSA. Backwards-compatibility and not breaking any existing setups is a must, I agree. The PR, as I left it, passes all existing tests (barring a help documentation one, unrelated to the json/settings stuff). All existing psd1/hashtable usages for I've not touched the PR in a while as I'm still learning/improving with C# - I'm not sure about the abstractions/structure I've put in place for settings. If you ever felt like giving it a high-level look and giving me some general guidance - I'd really appreciate that - though I know you're a super busy person, so no worries if not. My motivations for this PR actually focus around discoverability of rules and their settings, as well as for my own fun/learning on how settings were implemented in PSSA. Currently you can use The PSD1 settings file experience isn't amazing. I appreciate that if you're a PowerShell person then the syntax is familiar to you - it's just a hashtable after all. But that doesn't help you with the structure; there's no tooling to offer validation of a settings file beyond the parser validating that it's legal PowerShell. Imagine supporting a JSON settings format: We could generate and ship a JSON schema alongside the module. The VS Code extension could use that JSON Schema to offer intelisense and validation with very minimal effort I would hope. If it were published somewhere then most other popular IDEs could do the same too. I notice that |
PR Summary
Very much a work in progress, but almost all tests pass (new cmdlet needs help for
Get-Help) and json files are usable wherever .psd1 settings file can be used.Json is a widely used format with lots of great existing tooling. It could offer some nice UX in future (could publish JSON Schemas to offer inline validation in tools such as VS Code etc).
Along with changes aimed at making Json (and potentially other formats) viable I wanted settings to be more discoverable. To that end I've:
Proposed a new cmdlet
New-ScriptAnalyzerSettingsFilewhich would allow creation of settings file in desired format. One option is-Allwhich populates file with all rules and all configurable rule options (set to their defaults).Added a list of options to the
RuleInfoclass. For configurable built-in rules, the options, their types, and default value are populated.Additional change detail:
Internal location of Settings Presets moved from
Engine\Settings->Engine\SettingsPresetssoSettings folder can be used for all Settings logic and parsers. Presets are still shipped in the
built module's
Settingsfolder.Internal location of Command Data Files moved from
Engine\Settings->Engine\CommandDataFiles.Now separated out from the Settings Presets, as presets could now be in
.jsonformat - which iswhat previously distinguished the command data files from presets. They are shipped with the
module in a new folder
CommandDataFiles.GetShippedSettingsDirectory()abstracted toGetShippedModuleSubDirectory(string subDirectoryName)as there are now two folders,
Settings, andCommandDataFiles, which consumers may care about.Usages updated in
AvoidOverwritingBuiltInCmdletsandUseCompatibleCmdletswhich load and usethe Command Data Files.
All state about settings data moved to
SettingsDataclass.Settingsclass becomes a purestatic helper. It's responsible for the supported file formats, preset resolution, auto-discovery,
and orchestrating the deserialization of a settings file into a
SettingsDataand vice-versa.An interface
ISettingsFormatdescribes a Settings Format. It can serialize aSettingsDatato astring representation of the intended format and deserialize that string representation into a
SettingsData. 2 Implementations of this arePsd1SettingsFormatandJsonSettingsFormat.The order of the Settings Formats in the
Settingsclass (s_formats) defines the order in whichformats are checked during Auto-Discovery and Preset Resolution. For instance if
JsonSettingsFormatprecedes
Psd1SettingsFormat:During Auto-Discovery, if both
PSScriptAnalyzerSettings.jsonandPSScriptAnalyzerSettings.psd1exist,
PSScriptAnalyzerSettings.jsonwill be used.During Preset resolution, if the preset
CmdletDesignis used and bothCmdletDesign.jsonandCmdletDesign.psd1exist in the presets folder, the settings inCmdletDesign.jsonwould beused. Note: In practice, this should be avoided. A test to ensure preset uniqueness should
be added. Presets should only be shipped in a single format.
When passing values in via
-Settings:When nothing supplied and no settings file in cwd:
Same as current behaviour. Auto discovery fails. Built-in defaults used
When nothing supplied and a psd1 settings file in cwd:
Different from current which omits the path. Note the space between path and period below
which should contain the cwd path.
When nothing supplied and a json settings file in cwd:
Different from current which doesn't know about the json format.
When incorrect type supplied:
Differs from current behaviour which proceeds with built-in settings, noting only that settings
cannot be found. This is inconsistent with when an invalid path is supplied when execution
doesn't proceed.
When Preset name supplied:
Differs from current behaviour - calls out that a preset was used.
When explicit valid psd1 file path supplied:
Same as current behaviour.
When explicit valid json file path supplied:
Different from current behaviour which fails with:
When explicit invalid file path supplied:
Same as current behaviour.
New Cmdlet
New-ScriptAnalyzerSettingsFile. Intent is to make it easy to create settings files and discover available settings/options without having to directly resort to docs. It creates settings files based on presets (orAllavailable settings set to their defaults). Creates file in nominated format at desired path. Path defaults to cwd and file format defaults to highest precedence file format inSettings.cs(currently.json). Has argument completers for FileFormat and Base.Gist of
jsonandpsd1files created by cmdlet here.Still to look at:
SettingsDataobject for equivalent input.PR Checklist
.cs,.ps1and.psm1files have the correct copyright headerWIP:to the beginning of the title and remove the prefix when the PR is ready.