PowerShell / PowerShell/PSScriptAnalyzer

New formatting API

オープン
#1,550 コメント 3 件 リアクション 1 件 担当者 0 名 GitHub で見る

まだ誰も着手していません。

API Proposal Consider - 2.0 Issue - Discussion
主要言語
C#
スター
2.2k
フォーク
414
平均マージ
13時間 1分
マージ済み PR(30日)
2

説明

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

コントリビューションガイド

コントリビューションガイドを開く

はじめの一歩

  1. issue を最後まで読み、次にプロジェクトのコントリビューションガイドを読みます。
  2. 着手することを issue にコメントします — 二人が同じ作業をするのを防げます。
  3. リポジトリをフォークし、ブランチを切って変更します。
  4. issue 番号を参照したプルリクエストを送ります。

調査の方向性

まず、issue で提案されている ScriptEditor、ScriptFormatter、ScriptRule、IScriptEditor、IFormatStream のエントリーポイントを確認します。実装前に API とフォーマット戦略の設計を確定してください。編集追跡、ルール統合、入力およびファイルのフォーマットを網羅した合意済みの設計ができれば完了です。

索引モデルが issue の本文から書いたものです。

評価

技術スタック
csharp, powershell
領域
devtools, tooling
issue の種類
機能追加
難易度
5/5
見積もり時間
1週間以上
活発さ
停滞
明瞭さ
説明が足りない
初心者へのやさしさ
25/100

新しい issue をメールで受け取る

初心者向けの GitHub issue を短くまとめたダイジェスト。