PowerShell / PowerShell/PSScriptAnalyzer

New formatting API

Ouverte
#1,550 3 commentaires 1 réaction 0 personnes assignées Voir sur GitHub

Personne n'a encore pris cette issue.

API Proposal Consider - 2.0 Issue - Discussion
Langage dominant
C#
Étoiles
2.2k
Forks
414
Merge moyen
13 h 1 min
PR mergées (30 j)
2

Description

Currently in PSScriptAnalyzer, script formatting occurs by writing a ScriptRule that emits corrections. Formatting is done by running rules one at a time, applying the first of any corrections on each diagnostic, re-assembling and re-parsing the script and running again until all rules have been run.

Looking at this, we should be able to improve the efficiency of formatting significantly with a good implementation, but ideally the design does not constrain the implementation beyond what is essential to the domain problem. Therefore a formatting design should:

  • Not depend on any representation of the script beyond those natural to PowerShell; formatters should depend on the string representation, AST, etc and not think about buffers or string builders
  • Be agnostic of concurrency; formatters should not need to consider what else is happening while they format scripts
  • Not require allocation for each formatting edit; implementations may be able to amortise allocations for formats, so forcing an object to be allocated each time adds unnecessary overhead
  • Track what class supplied what formatting edit, so that edits can be filtered or otherwise managed on a per-provider basis
  • Ideally still allow rules to implement formatters

A simple proposal for this is:

public abstract class ScriptEditor
{
    protected ScriptEditor(ScriptEditorInfo editorInfo)
    {
        EditorInfo = editorInfo;
    }

    public ScriptEditorInfo EditorInfo { get; }

    // The formatting engine would call this to run the editor
    public abstract void Run(string scriptContent, Ast ast, IReadOnlyList<Token> tokens, string scriptFilePath);

    // This method is how implementers edit scripts
    // We can easily add overloads that accept IScriptPositions, IScriptExtents, Asts, Tokens, even line/column combinations
    protected void Replace(int startOffset, int endOffset, ReadOnlySpan<char> newValue)
    {
        ...
    }
}

This could then be encapsulated into an object that actually runs the editors as a formatter:

// This might also be presented to consumers as an IScriptFormatter implemented by an internal class created with a builder
// for added flexibility in the API
public class ScriptFormatter
{
    // Run inside implementation
    private readonly IReadOnlyList<ScriptEditor> _scriptEditors;

    public string FormatInput(string scriptInput)
    {
        ...
    }

    public string FormatFile(string scriptFilePath) => FormatFile(inputFile: scriptFilePath, outputFile: scriptFilePath);

    public string FormatFile(string inputFile, string outputFile)
    {
        ...
    }
}

This design is nice because:

  • It limits the number of types/concepts required for an implementer to one; implement the abstract method and call the protected method
  • Things like the ScriptEditorInfo object can be passed to the formatter implicitly, without requiring the implementer to hook anything up or opening the possibility that they will hook things up wrong
  • The ScriptFormatter object and ScriptEditor base class are free to choose an implementation, whether that serially runs, applies and reparses rules
    or concurrently runs them, reifies the edits into objects, applies them serially in order, and then reruns those that couldn't be applied (this kind of choice could also be captured by something like an IScriptFormattingStrategy, to generalise the relationship between ScriptFormatter and ScriptEditor)

The main drawback is that it's less simple to implement a rule and a formatter in the same class. One way to accomplish this might be a compositional approach, where rules declare themselves to also be formatters with an attribute or by implementing an interface, and then we have a ScriptEditor implementation that uses those rules internally:

internal class RuleScriptEditor : ScriptEditor
{
    private readonly ScriptRule _rule;

    public RuleScriptEditor(ScriptRule rule)
    {
        _rule = rule;
    }

    public override void Run(...)
    {
        _rule.Run(...);
        ...
        // Collect diagnostics from rule and use them to call the edit API
    }
}

Doing this might enable the traditional approach of rules that format, while encouraging the decoupling of the two functions into two more cohesive APIs.

A final alternative to this is to make formatting APIs purely interface-based:

public interface IScriptEditor
{
    void Run(IFormatStream stream, ...);
}

public interface IFormatStream
{
    // Possibly IScriptEditor in the first argument
    void Replace(ScriptEditorInfo editorInfo, int startOffset, int endOffset, ReadOnlySpan<char> newValue);
}

public static class ScriptEditorExtensions
{
    // Convenience APIs provided to make API use simpler and require less redundancy
}

The big problems here are:

  • We now require implementers to know about three classes rather than one
  • An implementer must write more boilerplate about the editor info object and could pass through a bad value of editorInfo in the first argument and cause issues for the engine
  • We have no simple way to inject a ScriptEditorInfo object into implementing classes so we can enforce nice uniqueness/referential equality properties

Guide de contribution

Ouvrir le guide de contribution

Par où commencer

  1. Lisez l'issue en entier, puis le guide de contribution du projet.
  2. Signalez en commentaire que vous la prenez — cela évite que deux personnes fassent le même travail.
  3. Forkez le dépôt et travaillez sur une branche.
  4. Ouvrez une pull request qui référence le numéro de l'issue.

Piste de recherche

Commencez par examiner les points d’entrée proposés dans l’issue : ScriptEditor, ScriptFormatter, ScriptRule, IScriptEditor et IFormatStream. Définissez la conception de l’API et de la stratégie de formatage avant l’implémentation ; le travail est considéré comme terminé lorsqu’une conception approuvée couvre le suivi des modifications, l’intégration des règles et le formatage des entrées et des fichiers.

Rédigé par le modèle d'indexation à partir du texte de l'issue.

Évaluation

Stack technique
csharp, powershell
Domaine
devtools, tooling
Type d'issue
Fonctionnalité
Difficulté
5/5
Temps estimé
Plus d'une semaine
Activité
À l'abandon
Clarté
À clarifier
Accessibilité débutants
25/100

Recevez les nouvelles issues par e-mail

Un résumé court des issues GitHub adaptées aux débutants.