- 
                Notifications
    You must be signed in to change notification settings 
- Fork 403
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
          
     Draft
      
      
            liamjpeters
  wants to merge
  12
  commits into
  PowerShell:main
  
    
      
        
          
  
    
      Choose a base branch
      
     
    
      
        
      
      
        
          
          
        
        
          
            
              
              
              
  
           
        
        
          
            
              
              
           
        
       
     
  
        
          
            
          
            
          
        
       
    
      
from
liamjpeters:SettingsRefactor
  
      
      
   
  
    
  
  
  
 
  
      
    base: main
Could not load branches
            
              
  
    Branch not found: {{ refName }}
  
            
                
      Loading
              
            Could not load tags
            
            
              Nothing to show
            
              
  
            
                
      Loading
              
            Are you sure you want to change the base?
            Some commits from the old base branch may be removed from the timeline,
            and old review comments may become outdated.
          
          
                
     Draft
            
            
          Conversation
  
    
      This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
      Learn more about bidirectional Unicode characters
    
  
  
    
    …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)
  
    Sign up for free
    to join this conversation on GitHub.
    Already have an account?
    Sign in to comment
  
      
  Add this suggestion to a batch that can be applied as a single commit.
  This suggestion is invalid because no changes were made to the code.
  Suggestions cannot be applied while the pull request is closed.
  Suggestions cannot be applied while viewing a subset of changes.
  Only one suggestion per line can be applied in a batch.
  Add this suggestion to a batch that can be applied as a single commit.
  Applying suggestions on deleted lines is not supported.
  You must change the existing code in this line in order to create a valid suggestion.
  Outdated suggestions cannot be applied.
  This suggestion has been applied or marked resolved.
  Suggestions cannot be applied from pending reviews.
  Suggestions cannot be applied on multi-line comments.
  Suggestions cannot be applied while the pull request is queued to merge.
  Suggestion cannot be applied right now. Please check back later.
  
    
  
    
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.