PowerShell / PowerShell/PSScriptAnalyzer

rule request: AvoidPlusEqualToPopulateDictionary

Abierto
#2,154 0 comentarios 2 reacciones 0 asignados Ver en GitHub

Nadie ha tomado este issue todavía.

Lenguaje dominante
C#
Estrellas
2.2k
Forks
414
Merge medio
13 h 1 min
PR fusionados (30 d)
2

Descripción

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

Guía de contribución

Abrir la guía de contribución

Primeros pasos

  1. Lee el issue completo y luego la guía de contribución del proyecto.
  2. Comenta en el issue que vas a ocuparte — evita que dos personas hagan lo mismo.
  3. Haz un fork del repositorio y trabaja en una rama.
  4. Abre un pull request que haga referencia al número del issue.

Línea de trabajo

El issue no nombra archivos de implementación ni pruebas. Empieza localizando las reglas existentes para += y sus patrones de prueba; después, verifica cómo una regla distinguiría += seguido de una tabla hash con una sola entrada y una clave dinámica. Se considera terminado cuando la nueva regla informa sobre la forma específica de población del diccionario y conserva las alternativas documentadas de corchetes, punto y Add.

Escrito por el modelo de indexación a partir del texto del issue.

Evaluación

Stack tecnológico
csharp, powershell
Área
devtools, tooling
Tipo de issue
Nueva funcionalidad
Dificultad
4/5
Tiempo estimado
3-5 días
Estado de actividad
Estancado
Claridad
Bastante claro
Aptitud para principiantes
45/100

Recibe los nuevos issues en tu correo

Un resumen breve de issues de GitHub para principiantes.