use pre-commit to invoke linters

.github/ruff.toml

@@ -1,10 +1,53 @@
-# 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_freeze.py" = ["F401", "F811"]
+"tests/test_function_id.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/
 

.pre-commit-config.yaml

@@ -0,0 +1,119 @@
+# 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
+ # ruff is fast, so run during commit
+ 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
+ # flake8 is kinda slow, so only run upon push, or like:
+ #
+ # pre-commit run flake8 --hook-stage push
+ stages: [commit, push]
+ language: system
+ entry: flake8
+ args: 
+ - "--max-line-length=120"
+ # E203: whitespace before ':' (black does this)
+ # F401: `foo` imported but unused (prefer ruff)
+ # F811 Redefinition of unused `foo` (prefer ruff)
+ # E501 line too long (prefer black)
+ - "--extend-ignore=E203,F401,F811,E501"
+ - "--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

capa/features/extractors/binja/basicblock.py

@@ -15,7 +15,6 @@
 from binaryninja import BasicBlock as BinjaBasicBlock
 from binaryninja import (
  BinaryView,
- DataBuffer,
  SymbolType,
  RegisterValueType,
  VariableSourceType,
@@ -26,7 +25,7 @@
 )
 
 from capa.features.common import Feature, Characteristic
-from capa.features.address import Address, AbsoluteVirtualAddress
+from capa.features.address import Address
 from capa.features.basicblock import BasicBlock
 from capa.features.extractors.helpers import MIN_STACKSTRING_LEN
 from capa.features.extractors.base_extractor import BBHandle, FunctionHandle

capa/features/extractors/binja/file.py

@@ -10,7 +10,7 @@
 import struct
 from typing import Tuple, Iterator
 
-from binaryninja import Symbol, Segment, BinaryView, SymbolType, SymbolBinding
+from binaryninja import Segment, BinaryView, SymbolType, SymbolBinding
 
 import capa.features.extractors.common
 import capa.features.extractors.helpers

capa/features/extractors/binja/function.py

@@ -19,7 +19,6 @@
 def extract_function_calls_to(fh: FunctionHandle):
  """extract callers to a function"""
  func: Function = fh.inner
- bv: BinaryView = func.view
 
  for caller in func.caller_sites:
  # Everything that is a code reference to the current function is considered a caller, which actually includes

capa/features/extractors/binja/global_.py

@@ -1,10 +1,8 @@
 import logging
-import contextlib
 from typing import Tuple, Iterator
 
 from binaryninja import BinaryView
 
-import capa.features.extractors.elf
 from capa.features.common import OS, OS_MACOS, ARCH_I386, ARCH_AMD64, OS_WINDOWS, Arch, Feature
 from capa.features.address import NO_ADDRESS, Address
 

capa/features/extractors/binja/insn.py

@@ -6,7 +6,7 @@
 # is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 # See the License for the specific language governing permissions and limitations under the License.
 import sys
-from typing import Any, Dict, List, Tuple, Iterator, Optional
+from typing import Any, List, Tuple, Iterator, Optional
 
 from binaryninja import Function
 from binaryninja import BasicBlock as BinjaBasicBlock
@@ -18,12 +18,11 @@
  RegisterValueType,
  LowLevelILOperation,
  LowLevelILInstruction,
- InstructionTextTokenType,
 )
 
 import capa.features.extractors.helpers
 from capa.features.insn import API, MAX_STRUCTURE_SIZE, Number, Offset, Mnemonic, OperandNumber, OperandOffset
-from capa.features.common import MAX_BYTES_FEATURE_SIZE, THUNK_CHAIN_DEPTH_DELTA, Bytes, String, Feature, Characteristic
+from capa.features.common import MAX_BYTES_FEATURE_SIZE, Bytes, String, Feature, Characteristic
 from capa.features.address import Address, AbsoluteVirtualAddress
 from capa.features.extractors.binja.helpers import DisassemblyInstruction, visit_llil_exprs
 from capa.features.extractors.base_extractor import BBHandle, InsnHandle, FunctionHandle
@@ -73,7 +72,6 @@ def extract_insn_api_features(fh: FunctionHandle, bbh: BBHandle, ih: InsnHandle)
  example:
  call dword [0x00473038]
  """
- insn: DisassemblyInstruction = ih.inner
  func: Function = fh.inner
  bv: BinaryView = func.view
 
@@ -128,12 +126,9 @@ def extract_insn_number_features(
  example:
  push 3136B0h ; dwControlCode
  """
- insn: DisassemblyInstruction = ih.inner
  func: Function = fh.inner
- bv: BinaryView = func.view
 
  results: List[Tuple[Any[Number, OperandNumber], Address]] = []
- address_size = func.view.arch.address_size * 8
 
  def llil_checker(il: LowLevelILInstruction, parent: LowLevelILInstruction, index: int) -> bool:
  if il.operation == LowLevelILOperation.LLIL_LOAD:
@@ -171,7 +166,6 @@ def extract_insn_bytes_features(fh: FunctionHandle, bbh: BBHandle, ih: InsnHandl
  example:
  push offset iid_004118d4_IShellLinkA ; riid
  """
- insn: DisassemblyInstruction = ih.inner
  func: Function = fh.inner
  bv: BinaryView = func.view
 
@@ -220,7 +214,6 @@ def extract_insn_string_features(
  example:
  push offset aAcr ; "ACR > "
  """
- insn: DisassemblyInstruction = ih.inner
  func: Function = fh.inner
  bv: BinaryView = func.view
 
@@ -278,7 +271,6 @@ def extract_insn_offset_features(
  example:
  .text:0040112F cmp [esi+4], ebx
  """
- insn: DisassemblyInstruction = ih.inner
  func: Function = fh.inner
 
  results: List[Tuple[Any[Offset, OperandOffset], Address]] = []
@@ -364,7 +356,6 @@ def extract_insn_nzxor_characteristic_features(
  parse instruction non-zeroing XOR instruction
  ignore expected non-zeroing XORs, e.g. security cookies
  """
- insn: DisassemblyInstruction = ih.inner
  func: Function = fh.inner
 
  results = []
@@ -414,7 +405,6 @@ def extract_insn_peb_access_characteristic_features(
 
  fs:[0x30] on x86, gs:[0x60] on x64
  """
- insn: DisassemblyInstruction = ih.inner
  func: Function = fh.inner
 
  results = []
@@ -456,7 +446,6 @@ def extract_insn_segment_access_features(
  fh: FunctionHandle, bbh: BBHandle, ih: InsnHandle
 ) -> Iterator[Tuple[Feature, Address]]:
  """parse instruction fs or gs access"""
- insn: DisassemblyInstruction = ih.inner
  func: Function = fh.inner
 
  results = []
@@ -485,7 +474,6 @@ def extract_insn_cross_section_cflow(
  fh: FunctionHandle, bbh: BBHandle, ih: InsnHandle
 ) -> Iterator[Tuple[Feature, Address]]:
  """inspect the instruction for a CALL or JMP that crosses section boundaries"""
- insn: DisassemblyInstruction = ih.inner
  func: Function = fh.inner
  bv: BinaryView = func.view
 
@@ -509,7 +497,6 @@ def extract_function_calls_from(fh: FunctionHandle, bbh: BBHandle, ih: InsnHandl
 
  most relevant at the function scope, however, its most efficient to extract at the instruction scope
  """
- insn: DisassemblyInstruction = ih.inner
  func: Function = fh.inner
  bv: BinaryView = func.view
 
@@ -555,7 +542,6 @@ def extract_function_indirect_call_characteristic_features(
  most relevant at the function or basic block scope;
  however, its most efficient to extract at the instruction scope
  """
- insn: DisassemblyInstruction = ih.inner
  func: Function = fh.inner
 
  llil = func.get_llil_at(ih.address)