Merge pull request #1591 from mandiant/fix/issue-1579

use pre-commit to invoke linters

.github/flake8.ini

@@ -0,0 +1,29 @@
+[flake8]
+max-line-length = 120
+
+extend-ignore =
+ # E203: whitespace before ':' (black does this)
+ E203,
+ # F401: `foo` imported but unused (prefer ruff)
+ F401,
+ # F811 Redefinition of unused `foo` (prefer ruff)
+ F811,
+ # E501 line too long (prefer black)
+ E501,
+ # B010 Do not call setattr with a constant attribute value
+ B010,
+ # G200 Logging statement uses exception in arguments
+ G200
+
+
+per-file-ignores =
+ # T201 print found.
+ #
+ # scripts are meant to print output
+ scripts/*: T201
+ # capa.exe is meant to print output
+ capa/main.py: T201
+ # IDA tests emit results to output window so need to print
+ tests/test_ida_features.py: T201
+ # utility used to find the Binary Ninja API via invoking python.exe
+ capa/features/extractors/binja/find_binja_api.py: T201

.github/ruff.toml

@@ -1,10 +1,61 @@
-# Enable pycodestyle (`E`) codes
-select = ["E"]
+# Enable the pycodestyle (`E`) and Pyflakes (`F`) rules by default.
+# Unlike Flake8, Ruff doesn't enable pycodestyle warnings (`W`) or
+# McCabe complexity (`C901`) by default.
+select = ["E", "F"]
+
+# Allow autofix for all enabled rules (when `--fix`) is provided.
+fixable = ["ALL"]
+unfixable = []
 
 # E402 module level import not at top of file
 # E722 do not use bare 'except'
-ignore = ["E402", "E722"]
-exclude = ["*_pb2.py", "*_pb2.pyi"]
+# E501 line too long
+ignore = ["E402", "E722", "E501"]
+
+line-length = 120
+
+exclude = [
+ # Exclude a variety of commonly ignored directories.
+ ".bzr",
+ ".direnv",
+ ".eggs",
+ ".git",
+ ".git-rewrite",
+ ".hg",
+ ".mypy_cache",
+ ".nox",
+ ".pants.d",
+ ".pytype",
+ ".ruff_cache",
+ ".svn",
+ ".tox",
+ ".venv",
+ "__pypackages__",
+ "_build",
+ "buck-out",
+ "build",
+ "dist",
+ "node_modules",
+ "venv",
+ # protobuf generated files
+ "*_pb2.py", 
+ "*_pb2.pyi"
+]
 
-# Same as pycodestyle.
-line-length = 180
+[per-file-ignores]
+# until we address #1592 and move test fixtures into conftest.py
+# then we need to ignore imports done to enable pytest fixtures.
+#
+# F401: `foo` imported but unused
+# F811 Redefinition of unused `foo`
+"tests/test_main.py" = ["F401", "F811"]
+"tests/test_proto.py" = ["F401", "F811"]
+"tests/test_freeze.py" = ["F401", "F811"]
+"tests/test_function_id.py" = ["F401", "F811"]
+"tests/test_viv_features.py" = ["F401", "F811"]
+"tests/test_binja_features.py" = ["F401", "F811"]
+"tests/test_pefile_features.py" = ["F401", "F811"]
+"tests/test_dnfile_features.py" = ["F401", "F811"]
+"tests/test_dotnet_features.py" = ["F401", "F811"]
+"tests/test_result_document.py" = ["F401", "F811"]
+"tests/test_dotnetfile_features.py" = ["F401", "F811"]

.github/workflows/tests.yml

@@ -34,15 +34,15 @@ jobs:
  - name: Install dependencies
  run: pip install -e .[dev]
  - name: Lint with ruff
- run: ruff check --config .github/ruff.toml .
+ run: pre-commit run ruff
  - name: Lint with isort
- run: isort --profile black --length-sort --line-width 120 --skip-glob "*_pb2.py" -c .
+ run: pre-commit run isort
  - name: Lint with black
- run: black -l 120 --extend-exclude ".*_pb2.py" --check .
- - name: Lint with pycodestyle
- run: pycodestyle --exclude="*_pb2.py" --show-source capa/ scripts/ tests/
+ run: pre-commit run black
+ - name: Lint with flake8
+ run: pre-commit run flake8
  - name: Check types with mypy
- run: mypy --config-file .github/mypy/mypy.ini --check-untyped-defs capa/ scripts/ tests/
+ run:  pre-commit run mypy
 
  rule_linter:
  runs-on: ubuntu-20.04
@@ -56,7 +56,7 @@ jobs:
  with:
  python-version: "3.8"
  - name: Install capa
- run: pip install -e .
+ run: pip install -e .[dev]
  - name: Run rule linter
  run: python scripts/lint.py rules/
 

.gitignore

@@ -108,13 +108,10 @@ venv.bak/
 *.viv
 *.idb
 *.i64
+.vscode
+
 !rules/lib
 
-# hooks/ci.sh output
-isort-output.log
-black-output.log
-rule-linter-output.log
-.vscode
 scripts/perf/*.txt
 scripts/perf/*.svg
 scripts/perf/*.zip

.pre-commit-config.yaml

@@ -0,0 +1,111 @@
+# install the pre-commit hooks:
+#
+# ❯ pre-commit install --hook-type pre-commit
+# pre-commit installed at .git/hooks/pre-commit
+#
+# ❯ pre-commit install --hook-type pre-push
+# pre-commit installed at .git/hooks/pre-push
+#
+# run all linters liks:
+#
+# ❯ pre-commit run --all-files
+# isort....................................................................Passed
+# black....................................................................Passed
+# ruff.....................................................................Passed
+# flake8...................................................................Passed
+# mypy.....................................................................Passed
+#
+# run a single linter like:
+#
+# ❯ pre-commit run --all-files isort
+# isort....................................................................Passed
+
+repos:
+- repo: local
+ hooks:
+ - id: isort
+ name: isort
+ stages: [commit, push]
+ language: system
+ entry: isort
+ args: 
+ - "--length-sort"
+ - "--profile"
+ - "black"
+ - "--line-length=120"
+ - "--skip-glob"
+ - "*_pb2.py"
+ - "capa/"
+ - "scripts/"
+ - "tests/"
+ always_run: true
+ pass_filenames: false
+
+- repo: local
+ hooks:
+ - id: black
+ name: black
+ stages: [commit, push]
+ language: system
+ entry: black
+ args: 
+ - "--line-length=120"
+ - "--extend-exclude"
+ - ".*_pb2.py"
+ - "capa/"
+ - "scripts/"
+ - "tests/"
+ always_run: true
+ pass_filenames: false
+
+- repo: local
+ hooks:
+ - id: ruff
+ name: ruff
+ stages: [commit, push]
+ language: system
+ entry: ruff
+ args: 
+ - "check"
+ - "--config"
+ - ".github/ruff.toml"
+ - "capa/"
+ - "scripts/"
+ - "tests/"
+ always_run: true
+ pass_filenames: false
+
+- repo: local
+ hooks:
+ - id: flake8
+ name: flake8
+ stages: [commit, push]
+ language: system
+ entry: flake8
+ args: 
+ - "--config"
+ - ".github/flake8.ini"
+ - "--extend-exclude"
+ - "capa/render/proto/capa_pb2.py"
+ - "capa/"
+ - "scripts/"
+ - "tests/"
+ always_run: true
+ pass_filenames: false
+
+- repo: local
+ hooks:
+ - id: mypy
+ name: mypy
+ stages: [commit, push]
+ language: system
+ entry: mypy
+ args: 
+ - "--check-untyped-defs"
+ - "--ignore-missing-imports"
+ - "--config-file=.github/mypy/mypy.ini"
+ - "capa/"
+ - "scripts/"
+ - "tests/"
+ always_run: true
+ pass_filenames: false

CHANGELOG.md

@@ -6,6 +6,7 @@
 ### New Features
 - Utility script to detect feature overlap between new and existing CAPA rules [#1451](https://github.com/mandiant/capa/issues/1451) [@Aayush-Goel-04](https://github.com/aayush-goel-04)
 - use fancy box drawing characters for default output #1586 @williballenthin
+- use [pre-commit](https://pre-commit.com/) to invoke linters #1579 @williballenthin
 
 ### Breaking Changes
 - Update Metadata type in capa main [#1411](https://github.com/mandiant/capa/issues/1411) [@Aayush-Goel-04](https://github.com/aayush-goel-04) @manasghandat

capa/engine.py

@@ -8,7 +8,7 @@
 
 import copy
 import collections
-from typing import TYPE_CHECKING, Set, Dict, List, Tuple, Union, Mapping, Iterable, Iterator, cast
+from typing import TYPE_CHECKING, Set, Dict, List, Tuple, Union, Mapping, Iterable, Iterator
 
 import capa.perf
 import capa.features.common
@@ -71,7 +71,7 @@ def get_children(self) -> Iterator[Union["Statement", Feature]]:
  yield child
 
  if hasattr(self, "children"):
- for child in getattr(self, "children"):
+ for child in self.children:
  assert isinstance(child, (Statement, Feature))
  yield child
 
@@ -83,7 +83,7 @@ def replace_child(self, existing, new):
  self.child = new
 
  if hasattr(self, "children"):
- children = getattr(self, "children")
+ children = self.children
  for i, child in enumerate(children):
  if child is existing:
  children[i] = new

capa/features/common.py

@@ -100,7 +100,10 @@ def __nonzero__(self):
  return self.success
 
 
-class Feature(abc.ABC):
+class Feature(abc.ABC): # noqa: B024
+ # this is an abstract class, since we don't want anyone to instantiate it directly,
+ # but it doesn't have any abstract methods.
+
  def __init__(
  self,
  value: Union[str, int, float, bytes],
@@ -124,7 +127,12 @@ def __eq__(self, other):
  return self.name == other.name and self.value == other.value
 
  def __lt__(self, other):
- # TODO: this is a huge hack!
+ # implementing sorting by serializing to JSON is a huge hack.
+ # its slow, inelegant, and probably doesn't work intuitively;
+ # however, we only use it for deterministic output, so it's good enough for now.
+
+ # circular import
+ # we should fix if this wasn't already a huge hack.
  import capa.features.freeze.features
 
  return (
@@ -267,7 +275,7 @@ def __init__(self, substring: Substring, matches: Dict[str, Set[Address]]):
  self.matches = matches
 
  def __str__(self):
- matches = ", ".join(map(lambda s: '"' + s + '"', (self.matches or {}).keys()))
+ matches = ", ".join(f'"{s}"' for s in (self.matches or {}).keys())
  assert isinstance(self.value, str)
  return f'substring("{self.value}", matches = {matches})'
 
@@ -359,7 +367,7 @@ def __init__(self, regex: Regex, matches: Dict[str, Set[Address]]):
  self.matches = matches
 
  def __str__(self):
- matches = ", ".join(map(lambda s: '"' + s + '"', (self.matches or {}).keys()))
+ matches = ", ".join(f'"{s}"' for s in (self.matches or {}).keys())
  assert isinstance(self.value, str)
  return f"regex(string =~ {self.value}, matches = {matches})"