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

[ROU110] - Add a new rule for disallowing .save() without update fields or comments justifying it #18

Merged
merged 8 commits into from
Mar 8, 2024
Merged
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ Here is a list of the rules supported by this Flake8 plugin:
* `ROU107` - Inline function import is not at top of statement
* `ROU108` - Import from model module instead of sub-packages
* `ROU109` - Disallow rename migrations
* `ROU110` - Disallow .save() with no update_fields
* `ROU111` - Disallow FeatureFlag creation in code

## Testing
Expand Down
43 changes: 41 additions & 2 deletions flake8_routable.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
ROU107 = "ROU107 Inline function import is not at top of statement"
ROU108 = "ROU108 Import from model module instead of sub-packages"
ROU109 = "ROU109 Disallow rename migrations"
ROU110 = "ROU110 Disallow .save() with no update_fields"
ROU111 = "ROU111 Disallow FeatureFlag creation in code"


Expand Down Expand Up @@ -180,6 +181,7 @@ def visit(self, file_tokens: List[tokenize.TokenInfo]) -> None:
self.lines_with_invalid_docstrings()
self.lines_with_invalid_multi_line_strings()
self.rename_migrations()
self.disallow_no_update_fields_save()
self.disallow_feature_flag_creation()

def lines_with_blank_lines_after_comments(self) -> None:
Expand Down Expand Up @@ -349,14 +351,51 @@ def rename_migrations(self) -> None:
reported.add(line_token.start[0])
self.errors.append((*line_token.start, ROU109))

def disallow_no_update_fields_save(self) -> None:
""".save() must be called with update_fields."""
reported = set()
single_line_save = re.compile(r".+(\.save\(.*)")
allowed_comments = [
"# TODO: needs fix",
"# file save",
"# form save",
"# ledger save",
"# multi-line with update_fields",
"# new model save",
"# not a model",
"# save extension",
"# serializer save",
]

for line_token in self._file_tokens:
if line_token.start[0] in reported:
# There could be many tokens on a same line.
continue

line = line_token.line

if not single_line_save.match(line):
# Skip lines that don't match
continue

if "update_fields" in line:
# save, with update_fields is allowed
continue

if any(comment in line for comment in allowed_comments):
# Ignore lines with these comments, as they are valid
continue

reported.add(line_token.start[0])
self.errors.append((*line_token.start, ROU110))

def disallow_feature_flag_creation(self) -> None:
"""We can not create FeatureFlags in code, they are cached on the request."""
reported = set()
feature_flag_creation = re.compile(r".+(FeatureFlag\.objects\..*create)")
feature_flag_creation = re.compile(r"^.*?(FeatureFlag\.objects\..*create)")
allowed_comments = [
"# valid for legacy cross-border work",
"# valid for management command",
"# valid for test usage",
]

for line_token in self._file_tokens:
Expand Down
2 changes: 1 addition & 1 deletion setup.cfg
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
[metadata]
name = flake8_routable
version = 0.1.3
version = 0.1.4

[options]
py_modules = flake8_routable
Expand Down
122 changes: 122 additions & 0 deletions tests/test_flake8_routable.py
Original file line number Diff line number Diff line change
Expand Up @@ -536,6 +536,128 @@ def test_incorrect_import_from_tests(self):
assert errors == {"4:12: ROU109 Disallow rename migrations"}


class TestROU110:
SAVE_WITH_UPDATE_FIELDS = """from app.models import Model
instance = Model(id="123", name="test")
instance.save(update_fields=["id", "name"])
"""

SAVE_MULTILINE_WITH_UPDATE_FIELDS_FLAG = """from app.models import Model
instance = Model(id="123", name="test")
instance.save( # multi-line with update_fields
update_fields=["id", "name"]
)
"""

SAVE_WITHOUT_UPDATE_FIELDS = """from app.models import Model
instance = Model(id="123", name="test")
instance.save()
instance.save(using="default")
"""

SAVE_WITH_COMMENT = """from app.models import Model
instance = Model(id="123", name="test")
instance.save() {comment}
"""

def test_save_with_update_fields(self):
errors = results(self.SAVE_WITH_UPDATE_FIELDS)
assert errors == set()

def test_save__multiline_with_update_fields_flag(self):
errors = results(self.SAVE_MULTILINE_WITH_UPDATE_FIELDS_FLAG)
assert errors == set()

def test_save_without_update_fields(self):
errors = results(self.SAVE_WITHOUT_UPDATE_FIELDS)
assert errors == {
"3:0: ROU110 Disallow .save() with no update_fields",
"4:0: ROU110 Disallow .save() with no update_fields",
}

@pytest.mark.parametrize(
"comment",
[
"# TODO: needs fix",
"# file save",
"# form save",
"# ledger save",
"# new model save",
"# not a model",
"# save extension",
"# serializer save",
],
)
def test_with_comment(self, comment):
errors = results(self.SAVE_WITH_COMMENT.format(comment=comment))
assert errors == set()


class TestROU111:
FEATURE_FLAG_CREATE = """from feature_config.models import FeatureFlag
FeatureFlag.objects.create(company=company, feature_flag=flag)
"""

FEATURE_FLAG_CREATE_MULTILINE = """from feature_config.models import FeatureFlag
FeatureFlag.objects.create(
company=company, feature_flag=flag
)
"""

FEATURE_FLAG_GET_OR_CREATE = """from feature_config.models import FeatureFlag
def method():
FeatureFlag.objects.get_or_create(company=company, feature_flag=flag)
"""

FEATURE_FLAG_WITH_COMMENT = """from feature_config.models import FeatureFlag
FeatureFlag.objects.create( {comment}
company=company, feature_flag=flag
)
"""

def test_feature_flag_create(self):
errors = results(self.FEATURE_FLAG_CREATE)
assert errors == {
"2:0: ROU111 Disallow FeatureFlag creation in code",
}

def test_feature_flag_create_multiline(self):
errors = results(self.FEATURE_FLAG_CREATE_MULTILINE)
assert errors == {
"2:0: ROU111 Disallow FeatureFlag creation in code",
}

def test_feature_flag_get_or_create(self):
errors = results(self.FEATURE_FLAG_GET_OR_CREATE)
assert errors == {
"3:0: ROU111 Disallow FeatureFlag creation in code",
}

@pytest.mark.parametrize(
"comment",
[
"# valid for legacy cross-border work",
"# valid for management command",
],
)
def test_with_comment(self, comment):
errors = results(self.FEATURE_FLAG_WITH_COMMENT.format(comment=comment))
assert errors == set()

@pytest.mark.parametrize(
"comment",
[
"# valid for something else",
" ",
],
)
def test_with_invalid_comment(self, comment):
errors = results(self.FEATURE_FLAG_WITH_COMMENT.format(comment=comment))
assert errors == {
"2:0: ROU111 Disallow FeatureFlag creation in code",
}


class TestVisitor:
def test_parse_to_string_warning(self):
visitor = Visitor()
Expand Down
Loading