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.

Sign up for GitHub

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

Jump to bottom

Proposed graceful handling of noqa by L016 (#4248) #4424

Merged
merged 9 commits into from
Feb 26, 2023
Merged

Proposed graceful handling of noqa by L016 (#4248) #4424

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
9615e6f
graceful handling of noqa (#4248)
alanmcruickshank Feb 23, 2023
67f8f26
yamllint
alanmcruickshank Feb 23, 2023
3451465
Merge branch 'main' into ac/noqa_comments
alanmcruickshank Feb 23, 2023
360c89e
linting
alanmcruickshank Feb 23, 2023
575d665
Merge branch 'main' into ac/noqa_comments
alanmcruickshank Feb 24, 2023
68d340a
mypy
alanmcruickshank Feb 24, 2023
eaf70ca
Merge branch 'main' into ac/noqa_comments
alanmcruickshank Feb 25, 2023
5749dcb
Merge branch 'main' into ac/noqa_comments
alanmcruickshank Feb 25, 2023
6dc2ed9
Merge branch 'main' into ac/noqa_comments
alanmcruickshank Feb 26, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
149 changes: 84 additions & 65 deletions src/sqlfluff/utils/reflow/reindent.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1623,6 +1623,9 @@ def lint_line_length(
# the end of the line.
line_elements = line_buffer + [elem]

# Type hints
fixes: List[LintFix]

# Identify rebreak spans first so we can work out their indentation
# in the next section.
spans = identify_rebreak_spans(line_elements, root_segment)
Expand Down Expand Up @@ -1654,77 +1657,93 @@ def lint_line_length(
# We can only fix _inline_ comments in this way. Others should
# just be flagged as issues.
and line_buffer[-1].segments[-1].is_type("inline_comment")
and len(line_buffer[-1].segments[-1].raw) + len(current_indent)
<= line_length_limit
):
reflow_logger.debug(" Handling as inline comment line.")
comment_seg = line_buffer[-1].segments[-1]
# It is! Move the comment to the line before.
fixes = [
# Remove the comment from it's current position, and any whitespace
# in the previous point.
LintFix.delete(comment_seg),
*[
LintFix.delete(ws)
for ws in line_buffer[-2].segments
if ws.is_type("whitespace")
],
]
# Reinsert it at the start of the current line, with a newline after it.
if last_indent_idx:
fixes.append(
# NOTE: This looks a little convoluted, but we create *before*
# a block here rather than *after* a point, because the point
# may have been modified already by reflow code and may not be
# a reliable anchor.
LintFix.create_before(
elements[last_indent_idx + 1].segments[0],
[
comment_seg,
NewlineSegment(),
WhitespaceSegment(current_indent),
],
)

# Sense check a few edge cases:
if "noqa" in line_buffer[-1].segments[-1].raw:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved, but is this an ideal way to check noqa? Wouldn't this also fail on SELECT * FROM myschema.mynoqatable? Would we not want to verify type as a Comment?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah - good thought - but we've just confirmed that it's a comment on line 1659, so I think this is safe here.

reflow_logger.debug(" Unfixable because noqa unsafe to move.")
fixes = []
elif (
len(line_buffer[-1].segments[-1].raw) + len(current_indent)
> line_length_limit
):
reflow_logger.debug(
" Unfixable because comment too long anyway."
)
# Edge case handling for start of file:
fixes = []
else:
fixes.append(
LintFix.create_before(
first_seg,
comment_seg = line_buffer[-1].segments[-1]
# It is! Move the comment to the line before.
fixes = [
# Remove the comment from it's current position, and any
# whitespace in the previous point.
LintFix.delete(comment_seg),
*[
LintFix.delete(ws)
for ws in line_buffer[-2].segments
if ws.is_type("whitespace")
],
]
# Reinsert it at the start of the current line, with a newline
# after it.
if last_indent_idx:
fixes.append(
# NOTE: This looks a little convoluted, but we create
# *before* a block here rather than *after* a point,
# because the point may have been modified already by
# reflow code and may not be a reliable anchor.
LintFix.create_before(
elements[last_indent_idx + 1].segments[0],
[
comment_seg,
NewlineSegment(),
WhitespaceSegment(current_indent),
],
)
)
# Edge case handling for start of file:
else:
fixes.append(
LintFix.create_before(
first_seg,
[
comment_seg,
NewlineSegment(),
],
)
)
# Update the elements too (which is also a little complicated).
# everything up to this line
# + the comment
# + a new indent point
# + the rest of the line (without the last point and comment)
# + anything else after the line
if last_indent_idx is not None:
elements = (
elements[: last_indent_idx + 1]
+ [
line_buffer[-1],
ReflowPoint(
(
NewlineSegment(),
WhitespaceSegment(current_indent),
)
),
]
+ line_buffer[:-2]
+ elements[i:]
)
# Edge case for start of file:
else:
elements = (
[
comment_seg,
NewlineSegment(),
],
line_buffer[-1],
ReflowPoint((NewlineSegment(),)),
]
+ line_buffer[:-2]
+ elements[i:]
)
)
# Update the elements too (which is also a little complicated).
# everything up to this line
# + the comment
# + a new indent point
# + the rest of the line (without the last point and comment)
# + anything else after the line
if last_indent_idx is not None:
elements = (
elements[: last_indent_idx + 1]
+ [
line_buffer[-1],
ReflowPoint(
(NewlineSegment(), WhitespaceSegment(current_indent))
),
]
+ line_buffer[:-2]
+ elements[i:]
)
# Edge case for start of file:
else:
elements = (
[
line_buffer[-1],
ReflowPoint((NewlineSegment(),)),
]
+ line_buffer[:-2]
+ elements[i:]
)

# Then check for cases where we have no other options.
elif not matched_indents:
Expand Down
11 changes: 11 additions & 0 deletions test/fixtures/rules/std_rule_cases/L016.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -574,3 +574,14 @@ test_fix_window_function:
core:
max_line_length: 50
dialect: snowflake

test_fail_do_not_fix_noqa:
# https://github.com/sqlfluff/sqlfluff/issues/4248
# NOTE: No fix_str, because this should be unfixable.
fail_str: |
SELECT
col1,
col2,
col3
FROM
really_really_really_really_really_really_long_schema_name.TABLE1 -- noqa: L014