Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add pre-commit for sapp and fix lint error before #100

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions .github/pull_request_template.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,9 @@

**Pre-submission checklist**
- [ ] I've ran the following linters locally and fixed lint errors related to the files I modified in this PR
- [ ] `black .`
- [ ] `usort format .`
- [ ] `flake8`
1. `pip install pre-commit`(If you haven't install pre-commit yet)
2. `pre-commit install`(If you haven't install the pre-commit script yet.)
3. `pre-commit run`
- [ ] I've installed dev dependencies `pip install -r requirements-dev.txt` and completed the following:
- [ ] I've ran tests with `./scripts/run-tests.sh` and made sure all tests are passing

Expand Down
42 changes: 9 additions & 33 deletions .github/workflows/lint.yml
Original file line number Diff line number Diff line change
Expand Up @@ -9,44 +9,20 @@ on:
workflow_dispatch:

jobs:
black:
lint-with-precommit:
runs-on: ubuntu-latest

steps:
- uses: actions/checkout@v2
- uses: actions/setup-python@v2

- name: Install black
run: pip install black==22.3.0

- name: Run black
run: black --exclude 'stubs\/' --check --diff .

usort:
runs-on: ubuntu-latest

steps:
- uses: actions/checkout@v2
- uses: actions/setup-python@v2

- name: Install usort
run: pip install usort==1.0.2

- name: Run usort
run: usort diff . && usort check .

flake8:
runs-on: ubuntu-latest

steps:
- uses: actions/checkout@v2
- uses: actions/setup-python@v2
- uses: actions/checkout@v3
- uses: actions/setup-python@v4
with:
python-version: "3.10"

- name: Install flake8
run: pip install flake8
- name: Setup pre-commit
run: pip install pre-commit

- name: Run flake8
run: flake8
- name: Run pre-commit
run: pre-commit run --all-files

ESLint:
runs-on: ubuntu-latest
Expand Down
13 changes: 13 additions & 0 deletions .pre-commit-config.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
repos:
- repo: https://github.com/pycqa/flake8
rev: 6.1.0
hooks:
- id: flake8
- repo: https://github.com/facebook/usort
rev: v1.0.7
hooks:
- id: usort
- repo: https://github.com/psf/black-pre-commit-mirror
rev: 23.7.0
hooks:
- id: black
13 changes: 1 addition & 12 deletions sapp/db_support.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,18 +8,7 @@
import logging
from collections import namedtuple
from itertools import tee
from typing import (
Any,
Dict,
Iterable,
Iterator,
List,
Optional,
Set,
Tuple,
Type,
Union,
)
from typing import Dict, Iterable, Iterator, List, Optional, Set, Tuple, Type, Union

from munch import Munch
from sqlalchemy import and_, Column, exc, inspect, or_, String, types
Expand Down
1 change: 0 additions & 1 deletion sapp/filter.py
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,6 @@ def from_query(
max_trace_length_to_sources: Optional[int],
is_new_issue: Optional[bool],
) -> Filter:

restructured_features: List[Dict[str, Union[str, List[str]]]] = []
for filtering_condition in features or []:
feature_entry = {}
Expand Down
2 changes: 1 addition & 1 deletion sapp/json_diagnostics.py
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ def entries(self, search: str, pretty_print: bool = False) -> List[Dict[str, Any

entries = []
for callable_name, entry_location in entry_locations.items():
for (file_id, offset) in entry_location:
for file_id, offset in entry_location:
path = lookup_table.file_index[file_id]

errors = parser.get_json_from_file_offset(path, offset)
Expand Down
6 changes: 0 additions & 6 deletions sapp/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,6 @@ def process_result_value(self, value, dialect) -> IssueDBID:


class IssueInstanceTraceFrameAssoc(Base, PrepareMixin, RecordMixin):

__tablename__ = "issue_instance_trace_frame_assoc"
__table_args__ = BASE_TABLE_ARGS

Expand Down Expand Up @@ -1105,7 +1104,6 @@ def merge(cls, session, items):


class TraceFrameLeafAssoc(Base, PrepareMixin, RecordMixin):

__tablename__ = "trace_frame_message_assoc"
__table_args__ = BASE_TABLE_ARGS

Expand Down Expand Up @@ -1159,7 +1157,6 @@ class IssueInstanceFixInfo(Base, PrepareMixin, RecordMixin):


class TraceFrame(Base, PrepareMixin, RecordMixin):

__tablename__ = "trace_frames"
__table_args__ = (
Index("ix_traceframe_run_caller_port", "run_id", "caller_id", "caller_port"),
Expand Down Expand Up @@ -1325,7 +1322,6 @@ def type_intervals_match_or_ignored(
# of traces leading to some other leaf. TraceFrameAnnotationTraceFrameAssoc
# contains the first hop towards that leaf..
class TraceFrameAnnotation(Base, PrepareMixin, RecordMixin):

__tablename__ = "trace_frame_annotations"
__table_args__ = BASE_TABLE_ARGS

Expand Down Expand Up @@ -1395,7 +1391,6 @@ class TraceFrameAnnotation(Base, PrepareMixin, RecordMixin):
# IssueInstanceTraceFrameAssoc, which indicates the first hop trace frame from
# the issue instance.
class TraceFrameAnnotationTraceFrameAssoc(Base, PrepareMixin, RecordMixin):

__tablename__ = "trace_frame_annotation_trace_frame_assoc"
__table_args__ = BASE_TABLE_ARGS

Expand Down Expand Up @@ -1643,7 +1638,6 @@ class PrimaryKey(Base, PrimaryKeyBase):


class PrimaryKeyGenerator(PrimaryKeyGeneratorBase):

PRIMARY_KEY: Type = PrimaryKey

QUERY_CLASSES: Set[Type] = {
Expand Down
6 changes: 3 additions & 3 deletions sapp/pipeline/model_generator.py
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,7 @@ def _get_minimum_trace_length(
) -> int:
length = None
for entry in entries:
for (_leaf, depth) in entry.leaves:
for _leaf, depth in entry.leaves:
if length is None or length > depth:
length = depth
if length is not None:
Expand Down Expand Up @@ -439,7 +439,7 @@ def _generate_raw_trace_frame(

leaf_records = []
leaf_mapping_ids: Set[LeafMapping] = set()
for (leaf, depth) in leaves:
for leaf, depth in leaves:
leaf_record = self._get_shared_text(leaf_kind, leaf)
caller_leaf_id = self.graph.get_transform_normalized_caller_kind_id(
leaf_record
Expand Down Expand Up @@ -473,7 +473,7 @@ def _generate_raw_trace_frame(
reachability=FrameReachability.UNREACHABLE,
)

for (leaf_record, depth) in leaf_records:
for leaf_record, depth in leaf_records:
self.graph.add_trace_frame_leaf_assoc(trace_frame, leaf_record, depth)

# Note that the "graph._trace_frame_leaf_assoc" table is really associated with
Expand Down
1 change: 0 additions & 1 deletion sapp/tests/cli_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,6 @@ def test_previous_input(self, mock_analysis_output) -> None:


def assert_successful_exit(result: Result) -> None:

assert result.exit_code == 0, (
f"Command did not exit successfully: {result}\n"
# pyre-fixme
Expand Down
4 changes: 2 additions & 2 deletions sapp/tests/fake_object_generator.py
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ def precondition(
)
if self.graph:
self.graph.add_trace_frame(trace_frame)
for (leaf, depth) in leaves:
for leaf, depth in leaves:
# pyre-fixme[16]: `Optional` has no attribute
# `add_trace_frame_leaf_assoc`.
self.graph.add_trace_frame_leaf_assoc(trace_frame, leaf, depth)
Expand Down Expand Up @@ -171,7 +171,7 @@ def postcondition(
)
if self.graph:
self.graph.add_trace_frame(trace_frame)
for (leaf, depth) in leaves:
for leaf, depth in leaves:
# pyre-fixme[16]: `Optional` has no attribute
# `add_trace_frame_leaf_assoc`.
self.graph.add_trace_frame_leaf_assoc(trace_frame, leaf, depth)
Expand Down
2 changes: 1 addition & 1 deletion sapp/trace_graph.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
import logging
import re
from collections import defaultdict
from typing import Any, cast, DefaultDict, Dict, Iterable, List, Optional, Set, Tuple
from typing import cast, DefaultDict, Dict, Iterable, List, Optional, Set, Tuple

from .bulk_saver import BulkSaver
from .models import (
Expand Down
5 changes: 2 additions & 3 deletions sapp/trimmed_trace_graph.py
Original file line number Diff line number Diff line change
Expand Up @@ -365,7 +365,6 @@ def _next_interval(self, interval: Interval, next_frame: TraceFrame) -> Interval
def _recompute_trace_length_association(
self, visited: Visited, initial_frame_ids: Set[int], leaf_kind: SharedTextKind
) -> int:

"""Walks the traces starting at the initial frames with the initial
corresponding kinds to recompute and store the minimum trace length from each
reachable frame to the corresponding leaf."""
Expand Down Expand Up @@ -685,7 +684,7 @@ def _populate_issues_from_affected_conditions(

# Conditions that call this may have originated from other issues,
# keep searching for parent conditions leading to this one.
for (next_frame, frame_leaves) in self._get_predecessor_frames(
for next_frame, frame_leaves in self._get_predecessor_frames(
graph, leaves, condition
):
if len(frame_leaves) > 0:
Expand Down Expand Up @@ -786,7 +785,7 @@ def _add_trace_frame(self, graph: TraceGraph, trace_frame: TraceFrame) -> None:
self._populate_shared_text(graph, trace_frame.filename_id)
self._populate_shared_text(graph, trace_frame.caller_id)
self._populate_shared_text(graph, trace_frame.callee_id)
for (leaf_id, depth) in graph._trace_frame_leaf_assoc[trace_frame_id].items():
for leaf_id, depth in graph._trace_frame_leaf_assoc[trace_frame_id].items():
leaf = graph._shared_texts[leaf_id]
if leaf_id not in self._shared_texts:
self.add_shared_text(leaf)
Expand Down
1 change: 0 additions & 1 deletion sapp/ui/filters.py
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,6 @@ def all_filters(session: Session) -> List[Filter]:


def save_filter(session: Session, filter: Filter) -> None:

existing = (
session.query(FilterRecord).filter(FilterRecord.name == filter.name).first()
)
Expand Down
2 changes: 1 addition & 1 deletion sapp/ui/interactive.py
Original file line number Diff line number Diff line change
Expand Up @@ -774,7 +774,7 @@ def leaves(
query = queries.leaves(
session=session, kind=kind, run_id=self._current_run_id
)
for (_, name) in query:
for _, name in query:
leaves[name] += 1

results: Iterable[Tuple[str, int]]
Expand Down
Loading