PowerShell / PowerShell/PSScriptAnalyzer

rule request: AvoidPlusEqualToPopulateDictionary

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

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

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

説明

Avoid assignment by addition operator (+=) to populate a dictionary

As with using the assignment by addition operator (+=) to build a collection or a string, populating a dictionary using the assignment by addition operator (+=) is in effective:

Tests
$tests = @{
    'Array+=' = {
        param($count)

        $result = @()
        foreach($i in 1..$count) {
            $result += $i
        }
    }
    'IList.Add' = {
        param($count)

        $result = [Collections.Generic.List[int]]::new()
        foreach($i in 1..$count) {
            $result.Add($i)
        }
    }
    'IList+=' = {
        param($count)

        $result = [Collections.Generic.List[int]]::new()
        foreach($i in 1..$count) {
            $result += $i
        }
    }
    'IDictionary[]' = {
        param($count)

        $result = @{}
        foreach($i in 1..$count) {
            $result[$i] = $i
        }
    }
    'IDictionary.Add' = {
        param($count)

        $result = @{}
        foreach($i in 1..$count) {
            $result.Add($i, $i)
        }
    }

    'IDictionary+=' = {
        param($count)

        $result = @{}
        foreach($i in 1..$count) {
            $result += @{ $i = $i }
        }
    }
} # Tests

1kb, 5kb, 10kb | ForEach-Object {
    $groupResult = foreach($test in $tests.GetEnumerator()) {
        $ms = (Measure-Command { & $test.Value -Count $_ }).TotalMilliseconds

        [pscustomobject]@{
            CollectionSize    = $_
            Test              = $test.Key
            TotalMilliseconds = [Math]::Round($ms, 2)
        }

        [GC]::Collect()
        [GC]::WaitForPendingFinalizers()
    }

    $groupResult = $groupResult | Sort-Object TotalMilliseconds
    $groupResult | Select-Object *, @{
        Name       = 'RelativeSpeed'
        Expression = {
            $relativeSpeed = $_.TotalMilliseconds / $groupResult[0].TotalMilliseconds
            $speed = [Math]::Round($relativeSpeed, 2).ToString() + 'x'
            if ($speed -eq '1x') { $speed } else { $speed + ' slower' }
        }
    } | Format-Table -AutoSize
}
CollectionSize Test            TotalMilliseconds RelativeSpeed
-------------- ----            ----------------- -------------
          1024 IDictionary[]                0.19 1x
          1024 IList.Add                    0.35 1.84x slower
          1024 IDictionary.Add              0.50 2.63x slower
          1024 Array+=                      0.74 3.89x slower
          1024 IList+=                      1.16 6.11x slower
          1024 IDictionary+=               39.17 206.16x slower


CollectionSize Test            TotalMilliseconds RelativeSpeed
-------------- ----            ----------------- -------------
          5120 IDictionary[]                0.71 1x
          5120 IDictionary.Add              1.41 1.99x slower
          5120 IList.Add                    1.50 2.11x slower
          5120 IList+=                     20.33 28.63x slower
          5120 Array+=                     21.51 30.3x slower
          5120 IDictionary+=             1880.89 2649.14x slower


CollectionSize Test            TotalMilliseconds RelativeSpeed
-------------- ----            ----------------- -------------
         10240 IDictionary[]                0.92 1x
         10240 IDictionary.Add              3.79 4.12x slower
         10240 IList.Add                    4.71 5.12x slower
         10240 IList+=                     71.91 78.16x slower
         10240 Array+=                     86.93 94.49x slower
         10240 IDictionary+=             5502.40 5980.87x slower

Instead use (even for a single item):

  • the square bracket syntax ($Dictionary[$Key] = $Value) or dot syntax ($Dictionary.$Key = $Value) which will overwrite any duplicate key
  • the .Add() method ($Dictionary($Key, $Value)) which returns an error when the key already exists

Proposed technical implementation details (optional)

As apposed to the requests for avoiding the assignment by addition operator (+=) to build a collection or a string, the majority of this bad practice syntax would be easier to capture:

  • Check for the += operator
  • Directly followed by a hash table syntax @{ ... }
    • With a single key-value pair
    • And a dynamic key

Even using a simple search string will capture lot of issues (note that almost all results are embedded in a foreach loop):
https://github.com/search?q=language%3Apowershell+%22%2b=%20@%7B%20$%22&type=code

The LHS could still be anything else than a presumed IDictionary, e.g. an IList.
But than there is still a good reason to avoid the += operator...

What is the latest version of PSScriptAnalyzer at the point of writing

1.24.0

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

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

はじめの一歩

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

調査の方向性

この issue では実装ファイルもテストも指定されていません。まず += に対する既存のルールとそのテストパターンを探し、次に、動的なキーを持つ単一エントリのハッシュテーブルが += に続く場合を、ルールがどのように区別するかを確認してください。新しいルールが対象となる辞書追加の形式を報告し、ドキュメント化されている角括弧、ドット、Add の代替手段を維持できれば完了です。

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

評価

技術スタック
csharp, powershell
領域
devtools, tooling
issue の種類
機能追加
難易度
4/5
見積もり時間
3〜5日
活発さ
停滞
明瞭さ
おおむね明確
初心者へのやさしさ
45/100

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

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