Skip to content

Commit 3ea7107

Browse files
Copilotharupyclaude
authored
Consolidate comment tokenization in clint to a single pass per file (mlflow#22669)
Signed-off-by: harupy <17039389+harupy@users.noreply.github.com> Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: harupy <17039389+harupy@users.noreply.github.com> Co-authored-by: Claude <noreply@anthropic.com>
1 parent 767b663 commit 3ea7107

3 files changed

Lines changed: 54 additions & 49 deletions

File tree

‎dev/clint/src/clint/comments.py‎

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,6 @@ class Noqa:
2020

2121
@classmethod
2222
def from_token(cls, token: tokenize.TokenInfo) -> Self | None:
23-
# Import here to avoid circular dependency
2423
from clint.linter import Position
2524

2625
if match := NOQA_REGEX.match(token.string):
@@ -34,11 +33,9 @@ def from_token(cls, token: tokenize.TokenInfo) -> Self | None:
3433
def iter_comments(code: str) -> Iterator[tokenize.TokenInfo]:
3534
readline = io.StringIO(code).readline
3635
try:
37-
tokens = tokenize.generate_tokens(readline)
38-
for token in tokens:
36+
for token in tokenize.generate_tokens(readline):
3937
if token.type == tokenize.COMMENT:
4038
yield token
41-
4239
except tokenize.TokenError:
4340
# Handle incomplete tokens at end of file
4441
pass

‎dev/clint/src/clint/linter.py‎

Lines changed: 41 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
import tokenize
77
from dataclasses import dataclass
88
from pathlib import Path
9-
from typing import Any, Iterator, TypeAlias
9+
from typing import Any, Iterable, Iterator, TypeAlias
1010

1111
from typing_extensions import Self
1212

@@ -30,26 +30,28 @@ class DisableComment:
3030
column: int
3131
comment_line: int
3232

33-
34-
def parse_disable_comments(code: str) -> list[DisableComment]:
35-
"""Parses all `# clint: disable=` and `# clint: disable-next=` comments from source code."""
36-
result: list[DisableComment] = []
37-
readline = iter(code.splitlines(True)).__next__
38-
for tok in tokenize.generate_tokens(readline):
39-
if tok.type != tokenize.COMMENT:
40-
continue
41-
if m := DISABLE_COMMENT_REGEX.search(tok.string):
42-
is_next = m.group(1) is not None
43-
comment_line = tok.start[0] - 1
44-
target_line = comment_line + 1 if is_next else comment_line
45-
col = tok.start[1] + m.start()
46-
result.extend(
47-
DisableComment(
48-
rule=rule.strip(), line=target_line, column=col, comment_line=comment_line
49-
)
50-
for rule in m.group(2).split(",")
51-
)
52-
return result
33+
@classmethod
34+
def from_token(cls, token: tokenize.TokenInfo) -> list[Self]:
35+
if not (m := DISABLE_COMMENT_REGEX.search(token.string)):
36+
return []
37+
is_next = m.group(1) is not None
38+
comment_line = token.start[0] - 1
39+
target_line = comment_line + 1 if is_next else comment_line
40+
col = token.start[1] + m.start()
41+
return [
42+
cls(rule=rule.strip(), line=target_line, column=col, comment_line=comment_line)
43+
for rule in m.group(2).split(",")
44+
]
45+
46+
47+
def parse_comments(code: str) -> tuple[list[DisableComment], list[Noqa]]:
48+
disables: list[DisableComment] = []
49+
noqas: list[Noqa] = []
50+
for token in iter_comments(code):
51+
disables.extend(DisableComment.from_token(token))
52+
if noqa := Noqa.from_token(token):
53+
noqas.append(noqa)
54+
return disables, noqas
5355

5456

5557
HasLocation: TypeAlias = (
@@ -363,7 +365,7 @@ def __init__(
363365
*,
364366
path: Path,
365367
config: Config,
366-
disable_comments: list[DisableComment],
368+
disables: list[DisableComment],
367369
index: SymbolIndex,
368370
cell: int | None = None,
369371
offset: Position | None = None,
@@ -374,17 +376,17 @@ def __init__(
374376
Args:
375377
path: Path to the file being linted.
376378
config: Linter configuration declared within the pyproject.toml file.
377-
disable_comments: All disable comments found in the source code.
379+
disables: All disable comments found in the source code.
378380
index: Symbol index for resolving function signatures.
379381
cell: Index of the cell being linted in a Jupyter notebook.
380382
offset: Position offset to apply to the line and column numbers of the violations.
381383
"""
382384
self.stack: list[ast.FunctionDef | ast.AsyncFunctionDef | ast.ClassDef] = []
383385
self.path = path
384386
self.config = config
385-
self.disable_comments = disable_comments
387+
self.disables = disables
386388
self.ignore: dict[str, set[int]] = {}
387-
for dc in disable_comments:
389+
for dc in disables:
388390
self.ignore.setdefault(dc.rule, set()).add(dc.line)
389391
self.cell = cell
390392
self.violations: list[Violation] = []
@@ -532,18 +534,18 @@ def visit_example(
532534
except SyntaxError:
533535
return [Violation(rules.ExampleSyntaxError(), path, example.range)]
534536

535-
disable_comments = parse_disable_comments(example.code)
537+
disables, noqas = parse_comments(example.code)
536538
# Only track disable comments for rules checked in examples
537-
disable_comments = [dc for dc in disable_comments if dc.rule in config.example_rules]
539+
disables = [dc for dc in disables if dc.rule in config.example_rules]
538540
linter = cls(
539541
path=path,
540542
config=config,
541-
disable_comments=disable_comments,
543+
disables=disables,
542544
index=index,
543545
offset=example.range.start,
544546
)
545547
linter.visit(tree)
546-
linter.visit_comments(example.code)
548+
linter.visit_noqas(noqas)
547549
if index:
548550
v = ExampleVisitor(linter, index)
549551
v.visit(tree)
@@ -919,17 +921,16 @@ def post_visit(self) -> None:
919921
if range := self.lazy_modules.get(mod):
920922
self._check(range, rules.LazyModule())
921923

922-
for dc in self.disable_comments:
924+
for dc in self.disables:
923925
if (dc.rule, dc.line) not in self.used_disables:
924926
self._check(
925927
Range(Position(dc.comment_line, dc.column)),
926928
rules.UnusedDisableComment(dc.rule),
927929
)
928930

929-
def visit_comments(self, src: str) -> None:
930-
for comment in iter_comments(src):
931-
if noqa := Noqa.from_token(comment):
932-
self.visit_noqa(noqa)
931+
def visit_noqas(self, noqas: Iterable[Noqa]) -> None:
932+
for noqa in noqas:
933+
self.visit_noqa(noqa)
933934

934935
def visit_noqa(self, noqa: Noqa) -> None:
935936
if rule := rules.DoNotDisable.check(noqa.rules):
@@ -988,15 +989,16 @@ def _lint_cell(
988989
# Ignore non-python cells such as `!pip install ...`
989990
return violations
990991

992+
disables, noqas = parse_comments(src)
991993
linter = Linter(
992994
path=path,
993995
config=config,
994-
disable_comments=parse_disable_comments(src),
996+
disables=disables,
995997
index=index,
996998
cell=cell_index,
997999
)
9981000
linter.visit(tree)
999-
linter.visit_comments(src)
1001+
linter.visit_noqas(noqas)
10001002
linter.post_visit()
10011003
violations.extend(linter.violations)
10021004

@@ -1055,15 +1057,16 @@ def lint_file(path: Path, code: str, config: Config, index: SymbolIndex) -> list
10551057
violations.extend(Linter.visit_example(path, config, code_block, index))
10561058
return violations
10571059
else:
1060+
disables, noqas = parse_comments(code)
10581061
linter = Linter(
10591062
path=path,
10601063
config=config,
1061-
disable_comments=parse_disable_comments(code),
1064+
disables=disables,
10621065
index=index,
10631066
)
10641067
module = ast.parse(code)
10651068
linter.visit(module)
1066-
linter.visit_comments(code)
1069+
linter.visit_noqas(noqas)
10671070
linter.visit_file_content(code)
10681071
linter.post_visit()
10691072
return linter.violations

‎dev/clint/tests/test_ignore_map.py‎

Lines changed: 12 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,20 +1,25 @@
1-
from clint.linter import DisableComment, parse_disable_comments
1+
from clint.linter import DisableComment, parse_comments
2+
3+
4+
def _parse(code: str) -> list[DisableComment]:
5+
disables, _ = parse_comments(code)
6+
return disables
27

38

49
def test_single_rule() -> None:
510
code = """
611
x = 1 # clint: disable=rule-a
712
y = 2
813
"""
9-
assert parse_disable_comments(code) == [DisableComment("rule-a", 1, 9, 1)]
14+
assert _parse(code) == [DisableComment("rule-a", 1, 9, 1)]
1015

1116

1217
def test_multiple_rules() -> None:
1318
code = """
1419
x = 1 # clint: disable=rule-a,rule-b
1520
y = 2
1621
"""
17-
assert parse_disable_comments(code) == [
22+
assert _parse(code) == [
1823
DisableComment("rule-a", 1, 9, 1),
1924
DisableComment("rule-b", 1, 9, 1),
2025
]
@@ -25,7 +30,7 @@ def test_multiple_rules_with_spaces() -> None:
2530
x = 1 # clint: disable=rule-a, rule-b, rule-c
2631
y = 2
2732
"""
28-
assert parse_disable_comments(code) == [
33+
assert _parse(code) == [
2934
DisableComment("rule-a", 1, 9, 1),
3035
DisableComment("rule-b", 1, 9, 1),
3136
DisableComment("rule-c", 1, 9, 1),
@@ -38,7 +43,7 @@ def test_multiple_lines() -> None:
3843
y = 2 # clint: disable=rule-b
3944
z = 3 # clint: disable=rule-a,rule-b
4045
"""
41-
assert parse_disable_comments(code) == [
46+
assert _parse(code) == [
4247
DisableComment("rule-a", 1, 9, 1),
4348
DisableComment("rule-b", 2, 9, 2),
4449
DisableComment("rule-a", 3, 9, 3),
@@ -51,7 +56,7 @@ def test_no_disable_comments() -> None:
5156
x = 1
5257
y = 2
5358
"""
54-
assert parse_disable_comments(code) == []
59+
assert _parse(code) == []
5560

5661

5762
def test_various_spacing_around_commas() -> None:
@@ -61,7 +66,7 @@ def test_various_spacing_around_commas() -> None:
6166
c = 3 # clint: disable=rule-e ,rule-f
6267
d = 4 # clint: disable=rule-g , rule-h
6368
"""
64-
assert parse_disable_comments(code) == [
69+
assert _parse(code) == [
6570
DisableComment("rule-a", 1, 9, 1),
6671
DisableComment("rule-b", 1, 9, 1),
6772
DisableComment("rule-c", 2, 9, 2),

0 commit comments

Comments
 (0)