Merge pull request #52 from sr-lab/pyright

Add types and pyright

.github/workflows/lint.yml

@@ -18,4 +18,18 @@ jobs:
         uses: psf/black@stable
         with:
           options: "--check --verbose"
-          version: "23.3.0"
+          version: "23.3.0"
+
+      - name: Install Python 3
+        uses: actions/setup-python@v4
+        with:
+          python-version: 3.10.5
+
+      - name: Install dependencies pyright
+        run: |
+          python -m pip install --upgrade pip
+          pip install pyright
+          python -m pip install -e .
+
+      - name: Run pyright
+        run: pyright

glitch/__main__.py

@@ -1,39 +1,51 @@
 import click, os, sys
+
+from pathlib import Path
+from typing import Tuple, List, Set, Optional
 from glitch.analysis.rules import Error, RuleVisitor
 from glitch.helpers import RulesListOption, get_smell_types, get_smells
 from glitch.parsers.docker import DockerParser
 from glitch.stats.print import print_stats
 from glitch.stats.stats import FileStats
 from glitch.tech import Tech
 from glitch.repr.inter import UnitBlockType
+from glitch.parsers.parser import Parser
 from glitch.parsers.ansible import AnsibleParser
 from glitch.parsers.chef import ChefParser
 from glitch.parsers.puppet import PuppetParser
 from glitch.parsers.terraform import TerraformParser
 from pkg_resources import resource_filename
-from alive_progress import alive_bar
-from pathlib import Path
+from alive_progress import alive_bar  # type: ignore
+
 
 # NOTE: These are necessary in order for python to load the visitors.
 # Otherwise, python will not consider these types of rules.
-from glitch.analysis.design import DesignVisitor
-from glitch.analysis.security import SecurityVisitor
-
-
-def parse_and_check(type, path, module, parser, analyses, errors, stats):
+from glitch.analysis.design import DesignVisitor  # type: ignore
+from glitch.analysis.security import SecurityVisitor  # type: ignore
+
+
+def parse_and_check(
+    type: UnitBlockType,
+    path: str,
+    module: bool,
+    parser: Parser,
+    analyses: List[RuleVisitor],
+    errors: List[Error],
+    stats: FileStats,
+) -> None:
     inter = parser.parse(path, type, module)
     if inter != None:
         for analysis in analyses:
             errors += analysis.check(inter)
-    stats.compute(inter)
+        stats.compute(inter)
 
 
 @click.command(
     help="PATH is the file or folder to analyze. OUTPUT is an optional file to which we can redirect the smells output."
 )
 @click.option(
     "--tech",
-    type=click.Choice(Tech),
+    type=click.Choice([t.value for t in Tech]),
     required=True,
     help="The IaC technology in which the scripts analyzed are written in.",
 )
@@ -46,7 +58,7 @@ def parse_and_check(type, path, module, parser, analyses, errors, stats):
 )
 @click.option(
     "--type",
-    type=click.Choice(UnitBlockType),
+    type=click.Choice([t.value for t in UnitBlockType]),
     default=UnitBlockType.unknown,
     help="The type of scripts being analyzed.",
 )
@@ -97,19 +109,22 @@ def parse_and_check(type, path, module, parser, analyses, errors, stats):
 @click.argument("path", type=click.Path(exists=True), required=True)
 @click.argument("output", type=click.Path(), required=False)
 def glitch(
-    tech,
-    type,
-    path,
-    config,
-    module,
-    csv,
-    dataset,
-    includeall,
-    smell_types,
-    output,
-    tableformat,
-    linter,
+    tech: str,
+    type: str,
+    path: str,
+    config: str,
+    module: bool,
+    csv: bool,
+    dataset: bool,
+    includeall: Tuple[str, ...],
+    smell_types: Tuple[str, ...],
+    output: Optional[str],
+    tableformat: str,
+    linter: bool,
 ):
+    tech = Tech(tech)
+    type = UnitBlockType(type)
+
     if config != "configs/default.ini" and not os.path.exists(config):
         raise click.BadOptionUsage(
             "config", f"Invalid value for 'config': Path '{config}' does not exist."
@@ -138,40 +153,39 @@ def glitch(
     if smell_types == ():
         smell_types = get_smell_types()
 
-    analyses = []
+    analyses: List[RuleVisitor] = []
     rules = RuleVisitor.__subclasses__()
     for r in rules:
         if smell_types == () or r.get_name() in smell_types:
             analysis = r(tech)
             analysis.config(config)
             analyses.append(analysis)
 
-    errors = []
+    errors: List[Error] = []
     if dataset:
         if includeall != ():
-            iac_files = []
+            iac_files: Set[str] = set()
             for root, _, files in os.walk(path):
                 for name in files:
                     name_split = name.split(".")
                     if (
                         name_split[-1] in includeall
                         and not Path(os.path.join(root, name)).is_symlink()
                     ):
-                        iac_files.append(os.path.join(root, name))
-            iac_files = set(iac_files)
+                        iac_files.add(os.path.join(root, name))
 
             with alive_bar(
                 len(iac_files),
                 title=f"ANALYZING ALL FILES WITH EXTENSIONS {includeall}",
-            ) as bar:
+            ) as bar:  # type: ignore
                 for file in iac_files:
                     parse_and_check(
                         type, file, module, parser, analyses, errors, file_stats
                     )
                     bar()
         else:
             subfolders = [f.path for f in os.scandir(f"{path}") if f.is_dir()]
-            with alive_bar(len(subfolders), title="ANALYZING SUBFOLDERS") as bar:
+            with alive_bar(len(subfolders), title="ANALYZING SUBFOLDERS") as bar:  # type: ignore
                 for d in subfolders:
                     parse_and_check(
                         type, d, module, parser, analyses, errors, file_stats
@@ -180,7 +194,7 @@ def glitch(
 
         files = [f.path for f in os.scandir(f"{path}") if f.is_file()]
 
-        with alive_bar(len(files), title="ANALYZING FILES IN ROOT FOLDER") as bar:
+        with alive_bar(len(files), title="ANALYZING FILES IN ROOT FOLDER") as bar:  # type: ignore
             for file in files:
                 parse_and_check(
                     type, file, module, parser, analyses, errors, file_stats
@@ -212,7 +226,7 @@ def glitch(
         print_stats(errors, get_smells(smell_types, tech), file_stats, tableformat)
 
 
-def main():
+def main() -> None:
     glitch(prog_name="glitch")
 
 

glitch/analysis/design.py

@@ -1,16 +1,17 @@
-from cmath import inf
 import json
 import re
 import configparser
+
+from cmath import inf
 from glitch.analysis.rules import Error, RuleVisitor, SmellChecker
 from glitch.tech import Tech
-
 from glitch.repr.inter import *
+from typing import List, Tuple, Dict, Set
 
 
 class DesignVisitor(RuleVisitor):
     class ImproperAlignmentSmell(SmellChecker):
-        def check(self, element, file: str):
+        def check(self, element: CodeElement, file: str) -> List[Error]:
             if isinstance(element, AtomicUnit):
                 identation = None
                 for a in element.attributes:
@@ -36,7 +37,12 @@ class PuppetImproperAlignmentSmell(SmellChecker):
         cached_file = ""
         lines = []
 
-        def check(self, element, file: str) -> list[Error]:
+        def check(self, element: CodeElement, file: str) -> List[Error]:
+            if not isinstance(element, AtomicUnit) and not isinstance(
+                element, UnitBlock
+            ):
+                return []
+
             if DesignVisitor.PuppetImproperAlignmentSmell.cached_file != file:
                 with open(file, "r") as f:
                     DesignVisitor.PuppetImproperAlignmentSmell.lines = f.readlines()
@@ -81,17 +87,17 @@ def check(self, element, file: str) -> list[Error]:
 
     class AnsibleImproperAlignmentSmell(SmellChecker):
         # YAML does not allow improper alignments (it also would have problems with generic attributes for all modules)
-        def check(self, element: AtomicUnit, file: str):
+        def check(self, element: CodeElement, file: str) -> List[Error]:
             return []
 
     class MisplacedAttribute(SmellChecker):
-        def check(self, element, file: str):
+        def check(self, element: CodeElement, file: str) -> List[Error]:
             return []
 
     class ChefMisplacedAttribute(SmellChecker):
-        def check(self, element, file: str):
+        def check(self, element: CodeElement, file: str) -> List[Error]:
             if isinstance(element, AtomicUnit):
-                order = []
+                order: List[int] = []
                 for attribute in element.attributes:
                     if attribute.name == "source":
                         order.append(1)
@@ -111,7 +117,7 @@ def check(self, element, file: str):
             return []
 
     class PuppetMisplacedAttribute(SmellChecker):
-        def check(self, element, file: str):
+        def check(self, element: CodeElement, file: str) -> List[Error]:
             if isinstance(element, AtomicUnit):
                 for i, attr in enumerate(element.attributes):
                     if attr.name == "ensure" and i != 0:
@@ -161,15 +167,15 @@ def __init__(self, tech: Tech) -> None:
         else:
             self.comment = "//"
 
-        self.variable_stack = []
-        self.variables_names = []
+        self.variable_stack: List[int] = []
+        self.variables_names: List[str] = []
         self.first_code_line = inf
 
     @staticmethod
     def get_name() -> str:
         return "design"
 
-    def config(self, config_path: str):
+    def config(self, config_path: str) -> None:
         config = configparser.ConfigParser()
         config.read(config_path)
         DesignVisitor.__EXEC = json.loads(config["design"]["exec_atomic_units"])
@@ -179,7 +185,7 @@ def config(self, config_path: str):
         if "var_refer_symbol" not in config["design"]:
             DesignVisitor.__VAR_REFER_SYMBOL = None
         else:
-            DesignVisitor.__VAR_REFER_SYMBOL = json.loads(
+            DesignVisitor.__VAR_REFER_SYMBOL = json.loads(  # type: ignore
                 config["design"]["var_refer_symbol"]
             )
 
@@ -190,8 +196,8 @@ def check_module(self, m: Module) -> list[Error]:
         #     errors.append(Error('design_unnecessary_abstraction', m, m.path, repr(m)))
         return errors
 
-    def check_unitblock(self, u: UnitBlock) -> list[Error]:
-        def count_atomic_units(ub: UnitBlock):
+    def check_unitblock(self, u: UnitBlock) -> List[Error]:
+        def count_atomic_units(ub: UnitBlock) -> Tuple[int, int]:
             count_resources = len(ub.atomic_units)
             count_execs = 0
             for au in ub.atomic_units:
@@ -224,7 +230,7 @@ def count_atomic_units(ub: UnitBlock):
         for attr in u.attributes:
             self.variables_names.append(attr.name)
 
-        errors = []
+        errors: List[Error] = []
         # The order is important
         for au in u.atomic_units:
             errors += self.check_atomicunit(au, u.path)
@@ -254,7 +260,7 @@ def count_atomic_units(ub: UnitBlock):
                 error.line = i + 1
                 errors.append(error)
 
-        def count_variables(vars: list[Variable]):
+        def count_variables(vars: List[KeyValue]) -> int:
             count = 0
             for var in vars:
                 if isinstance(var.value, type(None)):
@@ -266,7 +272,7 @@ def count_variables(vars: list[Variable]):
         # The UnitBlock should not be of type vars, because these files are supposed to only
         # have variables
         if (
-            count_variables(u.variables) / max(len(code_lines), 1) > 0.3
+            count_variables(u.variables) / max(len(code_lines), 1) > 0.3  # type: ignore
             and u.type != UnitBlockType.vars
         ):
             errors.append(
@@ -293,12 +299,13 @@ def count_variables(vars: list[Variable]):
                                 error.line = i + 1
                                 errors.append(error)
 
-        def get_line(i, lines):
+        def get_line(i: int, lines: List[Tuple[int, int]]):
             for j, line in lines:
                 if i < j:
                     return line
+            raise RuntimeError("Line not found")
 
-        lines = []
+        lines: List[Tuple[int, int]] = []
         current_line = 1
         i = 0
         for c in all_code:
@@ -310,7 +317,7 @@ def get_line(i, lines):
                 i += 1
         lines.append((i, current_line))
 
-        blocks = {}
+        blocks: Dict[int, List[int]] = {}
         for i in range(len(code) - 150):
             hash = code[i : i + 150].__hash__()
             if hash not in blocks:
@@ -319,7 +326,7 @@ def get_line(i, lines):
                 blocks[hash].append(i)
 
         # Note: changing the structure to a set instead of a list increased the speed A LOT
-        checked = set()
+        checked: Set[int] = set()
         for _, value in blocks.items():
             if len(value) >= 2:
                 for i in value:
@@ -363,7 +370,9 @@ def check_atomicunit(self, au: AtomicUnit, file: str) -> list[Error]:
         errors += self.misplaced_attr.check(au, file)
 
         if au.type in DesignVisitor.__EXEC:
-            if "&&" in au.name or ";" in au.name or "|" in au.name:
+            if isinstance(au.name, str) and (
+                "&&" in au.name or ";" in au.name or "|" in au.name
+            ):
                 errors.append(
                     Error("design_multifaceted_abstraction", au, file, repr(au))
                 )
@@ -391,17 +400,15 @@ def check_atomicunit(self, au: AtomicUnit, file: str) -> list[Error]:
     def check_dependency(self, d: Dependency, file: str) -> list[Error]:
         return []
 
-    def check_attribute(
-        self, a: Attribute, file: str, au: AtomicUnit = None, parent_name: str = ""
-    ) -> list[Error]:
+    def check_attribute(self, a: Attribute, file: str) -> list[Error]:
         return []
 
     def check_variable(self, v: Variable, file: str) -> list[Error]:
         self.variables_names.append(v.name)
         return []
 
     def check_comment(self, c: Comment, file: str) -> list[Error]:
-        errors = []
+        errors: List[Error] = []
         if c.line >= self.first_non_comm_line:
             errors.append(Error("design_avoid_comments", c, file, repr(c)))
         return errors