Skip to content

Commit 100c628

Browse files
alanmcruickshankalanmcruickshank
authored
Build out rule and fix serialisation (#5364)
1 parent 44143cf commit 100c628

26 files changed

Lines changed: 1022 additions & 152 deletions

File tree

‎docs/source/releasenotes.rst‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,18 @@ This release makes a couple of potentially breaking changes:
2121
the python packaging configuration file (although keeping :code:`setuptools`
2222
as the default backend).
2323

24+
* The serialised output for :code:`sqlfluff lint` now contains more information
25+
about the span of linting issues and initial proposed fixes. Beside the *new*
26+
fields, the original fields of :code:`line_pos` and :code:`line_no` have been
27+
renamed to :code:`start_line_pos` and :code:`start_line_no`, to distinguish
28+
them from the new fields starting :code:`end_*`.
29+
30+
* The default :code:`annotation_level` set by the :code:`--annotation-level`
31+
option on the :code:`sqlfluff lint` command has been changed from :code:`notice`
32+
to :code:`warning`, to better distinguish linting errors from warnings, which
33+
always now have the level of :code:`notice`. This is only relevant when using
34+
the :code:`github-annotation`` or :code:`github-annotation-native` formats.
35+
2436
Upgrading to 2.3
2537
----------------
2638

‎plugins/sqlfluff-templater-dbt/test/templater_test.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -330,7 +330,7 @@ def test__dbt_templated_models_do_not_raise_lint_error(
330330
print(linted_file.tree.stringify())
331331
print("\n\n## VIOLATIONS:")
332332
for idx, v in enumerate(linted_file.violations):
333-
print(f" {idx}:{v.get_info_dict()}")
333+
print(f" {idx}:{v.to_dict()}")
334334

335335
violations = lnt.check_tuples()
336336
assert len(violations) == 0

‎src/sqlfluff/cli/commands.py‎

Lines changed: 38 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -545,11 +545,13 @@ def dump_file_payload(filename: Optional[str], payload: str) -> None:
545545
)
546546
@click.option(
547547
"--annotation-level",
548-
default="notice",
548+
default="warning",
549549
type=click.Choice(["notice", "warning", "failure", "error"], case_sensitive=False),
550550
help=(
551-
"When format is set to github-annotation or github-annotation-native, "
552-
"default annotation level (default=notice). failure and error are equivalent."
551+
'When format is set to "github-annotation" or "github-annotation-native", '
552+
'default annotation level (default="warning"). "failure" and "error" '
553+
"are equivalent. Any rules configured only as warnings will always come "
554+
'through with type "notice" regardless of this option.'
553555
),
554556
)
555557
@click.option(
@@ -662,12 +664,27 @@ def lint(
662664
github_result.append(
663665
{
664666
"file": filepath,
665-
"line": violation["line_no"],
666-
"start_column": violation["line_pos"],
667-
"end_column": violation["line_pos"],
667+
"start_line": violation["start_line_no"],
668+
"start_column": violation["start_line_pos"],
669+
# NOTE: There should always be a start, there _may_ not be an
670+
# end, so in that case we default back to just re-using
671+
# the start.
672+
"end_line": violation.get(
673+
"end_line_no", violation["start_line_no"]
674+
),
675+
"end_column": violation.get(
676+
"end_line_pos", violation["start_line_pos"]
677+
),
668678
"title": "SQLFluff",
669679
"message": f"{violation['code']}: {violation['description']}",
670-
"annotation_level": annotation_level,
680+
# The annotation_level is configurable, but will only apply
681+
# to any SQLFluff rules which have not been downgraded
682+
# to warnings using the `warnings` config value. Any which have
683+
# been set to warn rather than fail will always be given the
684+
# `notice` annotation level in the serialised result.
685+
"annotation_level": annotation_level
686+
if not violation["warning"]
687+
else "notice",
671688
}
672689
)
673690
file_output = json.dumps(github_result)
@@ -681,11 +698,22 @@ def lint(
681698
for violation in record["violations"]:
682699
# NOTE: The output format is designed for GitHub action:
683700
# https://docs.github.com/en/actions/using-workflows/workflow-commands-for-github-actions#setting-a-notice-message
684-
line = f"::{annotation_level} "
701+
702+
# The annotation_level is configurable, but will only apply
703+
# to any SQLFluff rules which have not been downgraded
704+
# to warnings using the `warnings` config value. Any which have
705+
# been set to warn rather than fail will always be given the
706+
# `notice` annotation level in the serialised result.
707+
line = "::notice " if violation["warning"] else f"::{annotation_level} "
708+
685709
line += "title=SQLFluff,"
686710
line += f"file={filepath},"
687-
line += f"line={violation['line_no']},"
688-
line += f"col={violation['line_pos']}"
711+
line += f"line={violation['start_line_no']},"
712+
line += f"col={violation['start_line_pos']}"
713+
if "end_line_no" in violation:
714+
line += f",endLine={violation['end_line_no']}"
715+
if "end_line_pos" in violation:
716+
line += f",endColumn={violation['end_line_pos']}"
689717
line += "::"
690718
line += f"{violation['code']}: {violation['description']}"
691719
if violation["name"]:

‎src/sqlfluff/core/errors.py‎

Lines changed: 65 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -10,13 +10,26 @@
1010
1111
https://stackoverflow.com/questions/49715881/how-to-pickle-inherited-exceptions
1212
"""
13-
from typing import TYPE_CHECKING, Any, Dict, List, Optional, Tuple, Type, Union
13+
from typing import TYPE_CHECKING, Any, Dict, List, Optional, Tuple, Type, Union, cast
1414

1515
if TYPE_CHECKING: # pragma: no cover
1616
from sqlfluff.core.parser import BaseSegment, PositionMarker
1717
from sqlfluff.core.rules import BaseRule, LintFix
1818

1919
CheckTuple = Tuple[str, int, int]
20+
SerializedObject = Dict[str, Union[str, int, bool, List["SerializedObject"]]]
21+
22+
23+
def _extract_position(segment: Optional["BaseSegment"]) -> Dict[str, int]:
24+
"""If a segment is present and is a literal, return it's source length."""
25+
if segment:
26+
position = segment.pos_marker
27+
assert position
28+
if position.is_literal():
29+
return position.to_source_dict()
30+
# An empty location is an indicator of not being able to accurately
31+
# represent the location.
32+
return {} # pragma: no cover
2033

2134

2235
class SQLBaseError(ValueError):
@@ -83,17 +96,18 @@ def desc(self) -> str:
8396

8497
return self.__class__.__name__ # pragma: no cover
8598

86-
def get_info_dict(self) -> Dict[str, Union[str, int]]:
99+
def to_dict(self) -> SerializedObject:
87100
"""Return a dict of properties.
88101
89102
This is useful in the API for outputting violations.
90103
"""
91104
return {
92-
"line_no": self.line_no,
93-
"line_pos": self.line_pos,
105+
"start_line_no": self.line_no,
106+
"start_line_pos": self.line_pos,
94107
"code": self.rule_code(),
95108
"description": self.desc(),
96109
"name": getattr(self, "rule").name if hasattr(self, "rule") else "",
110+
"warning": self.warning,
97111
}
98112

99113
def check_tuple(self) -> CheckTuple:
@@ -212,6 +226,19 @@ def __reduce__(
212226
self.warning,
213227
)
214228

229+
def to_dict(self) -> SerializedObject:
230+
"""Return a dict of properties.
231+
232+
This is useful in the API for outputting violations.
233+
234+
For parsing errors we additionally add the length of the unparsable segment.
235+
"""
236+
_base_dict = super().to_dict()
237+
_base_dict.update(
238+
**_extract_position(self.segment),
239+
)
240+
return _base_dict
241+
215242

216243
class SQLLintError(SQLBaseError):
217244
"""An error which occurred during linting.
@@ -264,6 +291,40 @@ def __reduce__(
264291
self.warning,
265292
)
266293

294+
def to_dict(self) -> SerializedObject:
295+
"""Return a dict of properties.
296+
297+
This is useful in the API for outputting violations.
298+
299+
For linting errors we additionally add details of any fixes.
300+
"""
301+
_base_dict = super().to_dict()
302+
_base_dict.update(
303+
fixes=[fix.to_dict() for fix in self.fixes],
304+
**_extract_position(self.segment),
305+
)
306+
# Edge case: If the base error doesn't have an end position
307+
# but we only have one fix and it _does_. Then use use that in the
308+
# overall fix.
309+
_fixes = cast(List[SerializedObject], _base_dict.get("fixes", []))
310+
if "end_line_pos" not in _base_dict and len(_fixes) == 1:
311+
_fix = _fixes[0]
312+
# If the mandatory keys match...
313+
if (
314+
_fix["start_line_no"] == _base_dict["start_line_no"]
315+
and _fix["start_line_pos"] == _base_dict["start_line_pos"]
316+
):
317+
# ...then hoist all the optional ones from the fix.
318+
for key in [
319+
"start_file_pos",
320+
"end_line_no",
321+
"end_line_pos",
322+
"end_file_pos",
323+
]:
324+
_base_dict[key] = _fix[key]
325+
326+
return _base_dict
327+
267328
@property
268329
def fixable(self) -> bool:
269330
"""Should this error be considered fixable?"""

‎src/sqlfluff/core/linter/linted_file.py‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,6 @@ def check_tuples(
8282
raises that error.
8383
"""
8484
vs: List[CheckTuple] = []
85-
v: SQLLintError
8685
for v in self.get_violations():
8786
if isinstance(v, SQLLintError):
8887
vs.append(v.check_tuple())
@@ -123,7 +122,7 @@ def get_violations(
123122
filter_warning: bool = True,
124123
warn_unused_ignores: bool = False,
125124
fixable: Optional[bool] = None,
126-
) -> list:
125+
) -> List[SQLBaseError]:
127126
"""Get a list of violations, respecting filters and ignore options.
128127
129128
Optionally now with filters.

‎src/sqlfluff/core/linter/linting_result.py‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -205,13 +205,16 @@ def as_records(self) -> List[dict]:
205205
"filepath": path,
206206
"violations": sorted(
207207
# Sort violations by line and then position
208-
(v.get_info_dict() for v in violations),
208+
(v.to_dict() for v in violations),
209209
# The tuple allows sorting by line number, then position, then code
210-
key=lambda v: (v["line_no"], v["line_pos"], v["code"]),
210+
key=lambda v: (v["start_line_no"], v["start_line_pos"], v["code"]),
211211
),
212212
}
213213
for LintedDir in self.paths
214-
for path, violations in LintedDir.violation_dict().items()
214+
for path, violations in LintedDir.violation_dict(
215+
# Keep the warnings
216+
filter_warning=False
217+
).items()
215218
if violations
216219
]
217220

‎src/sqlfluff/core/parser/markers.py‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
"""
55

66
from dataclasses import dataclass
7-
from typing import TYPE_CHECKING, Any, Optional, Tuple
7+
from typing import TYPE_CHECKING, Any, Dict, Optional, Tuple
88

99
from sqlfluff.core.helpers.slice import zero_slice
1010

@@ -245,3 +245,7 @@ def is_literal(self) -> bool:
245245
def source_str(self) -> str:
246246
"""Returns the string in the source at this position."""
247247
return self.templated_file.source_str[self.source_slice]
248+
249+
def to_source_dict(self) -> Dict[str, int]:
250+
"""Serialise the source position."""
251+
return self.templated_file.source_position_dict_from_slice(self.source_slice)

‎src/sqlfluff/core/rules/fix.py‎

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,8 @@
33
import logging
44
from itertools import chain
55
from typing import (
6+
Any,
7+
Dict,
68
Iterable,
79
List,
810
Optional,
@@ -129,6 +131,54 @@ def __repr__(self) -> str:
129131
f"@{self.anchor.pos_marker} {detail}>"
130132
)
131133

134+
def to_dict(self) -> Dict[str, Any]:
135+
"""Serialise this LintFix as a dict."""
136+
assert self.anchor
137+
_position = self.anchor.pos_marker
138+
assert _position
139+
_src_loc = _position.to_source_dict()
140+
if self.edit_type == "delete":
141+
return {
142+
"type": self.edit_type,
143+
"edit": "",
144+
**_src_loc,
145+
}
146+
elif self.edit_type == "replace" and self.is_just_source_edit():
147+
assert self.edit is not None
148+
assert len(self.edit) == 1
149+
assert len(self.edit[0].source_fixes) == 1
150+
_source_fix = self.edit[0].source_fixes[0]
151+
return {
152+
"type": self.edit_type,
153+
"edit": _source_fix.edit,
154+
**_position.templated_file.source_position_dict_from_slice(
155+
_source_fix.source_slice
156+
),
157+
}
158+
159+
# Otherwise it's a standard creation or a replace.
160+
seg_list = cast(List[BaseSegment], self.edit)
161+
_edit = "".join(s.raw for s in seg_list)
162+
163+
if self.edit_type == "create_before":
164+
# If we're creating _before_, the end point isn't relevant.
165+
# Make it the same as the start.
166+
_src_loc["end_line_no"] = _src_loc["start_line_no"]
167+
_src_loc["end_line_pos"] = _src_loc["start_line_pos"]
168+
_src_loc["end_file_pos"] = _src_loc["start_file_pos"]
169+
elif self.edit_type == "create_after":
170+
# If we're creating _after_, the start point isn't relevant.
171+
# Make it the same as the end.
172+
_src_loc["start_line_no"] = _src_loc["end_line_no"]
173+
_src_loc["start_line_pos"] = _src_loc["end_line_pos"]
174+
_src_loc["start_file_pos"] = _src_loc["end_file_pos"]
175+
176+
return {
177+
"type": self.edit_type,
178+
"edit": _edit,
179+
**_src_loc,
180+
}
181+
132182
def __eq__(self, other: object) -> bool:
133183
"""Compare equality with another fix.
134184

‎src/sqlfluff/core/templaters/base.py‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -471,6 +471,19 @@ def source_only_slices(self) -> List[RawFileSlice]:
471471
ret_buff.append(elem)
472472
return ret_buff
473473

474+
def source_position_dict_from_slice(self, source_slice: slice) -> Dict[str, int]:
475+
"""Create a source position dict from a slice."""
476+
start = self.get_line_pos_of_char_pos(source_slice.start, source=True)
477+
stop = self.get_line_pos_of_char_pos(source_slice.stop, source=True)
478+
return {
479+
"start_line_no": start[0],
480+
"start_line_pos": start[1],
481+
"start_file_pos": source_slice.start,
482+
"end_line_no": stop[0],
483+
"end_line_pos": stop[1],
484+
"end_file_pos": source_slice.stop,
485+
}
486+
474487

475488
class RawTemplater:
476489
"""A templater which does nothing.

‎src/sqlfluff/diff_quality_plugin.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,7 @@ def violations_batch(self, src_paths):
7171
else:
7272
for file in report:
7373
self.violations_dict[file["filepath"]] = [
74-
Violation(v["line_no"], v["description"])
74+
Violation(v["start_line_no"], v["description"])
7575
for v in file["violations"]
7676
]
7777
else:

0 commit comments

Comments
 (0)