PowerShell / PowerShell/PSScriptAnalyzer

rule request: AvoidPlusEqualToPopulateDictionary

Open
#2,154 0 comments 2 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
C#
Stars
2.2k
Forks
414
Avg merge
13h 1m
Merged PRs (30d)
2

Description

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

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

The issue names no implementation files or tests. Start by locating existing rules for += and their test patterns, then verify how a rule would distinguish += followed by a single-entry hash table with a dynamic key. Done means the new rule reports the targeted dictionary-population form while retaining the documented bracket, dot, and Add alternatives.

Written by the indexing model from the issue text.

Assessment

Tech stack
csharp, powershell
Domain
devtools, tooling
Issue type
Feature
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
45/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.