Rule L019: Ignore comma placement violations if the adjacent code is templated #2717
Conversation
| parsed.tree, [], dialect=cfg.get("dialect_obj"), templated_file=rendered[0] | ||
| ) | ||
| lint_result = linter.lint_string(sql, config=cfg, fname="<STR>") | ||
| lerrs = lint_result.violations |
There was a problem hiding this comment.
Fixing old tech debt that was breaking the new test: I changed this function to call the public lint_string() which runs the templater, parser, and linter in one step. The old code calls the rule directly which is hacky, and it was skipping the template-related filtering of lint results built into the linter, causing my new "pass" tests to fail.
Codecov Report
@@ Coverage Diff @@
## main #2717 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 163 163
Lines 12064 12067 +3
=========================================
+ Hits 12064 12067 +3
Continue to review full report at Codecov.
|
barrywhart
changed the title
barrywhart
changed the title
| @@ -168,7 +168,6 @@ def templated_slices( | |||
| raise ValueError( | |||
| 'templated_slices: "templated_file" parameter is required.' | |||
| ) # pragma: no cover | |||
| templated_slices = [] | |||
| if ( | ||
| stop >= slice_.templated_slice.start and start < slice_.templated_slice.stop | ||
| ) | ||
| if (stop > slice_.templated_slice.start and start < slice_.templated_slice.stop) |
There was a problem hiding this comment.
Fixed a boundary condition issue that was returning slices that it shouldn't
| """Override new operator.""" | ||
| return super(TemplatedFileSlices, cls).__new__(cls, raw_slices) | ||
| return super(TemplatedFileSlices, cls).__new__(cls, templated_slices) |
There was a problem hiding this comment.
Fix old copy-paste naming issue
| break | ||
| if select_if is None or select_if(slice_): | ||
| buff.append(slice_) | ||
| return TemplatedFileSlices(*buff, templated_file=self.templated_file) |
There was a problem hiding this comment.
Added this method, equivalent to the select() methods on Segments and RawFileSlices. Ended up not using it, but thought it was worth keeping for future use.
| if ( | ||
| last_seg | ||
| and last_seg.is_type("newline") | ||
| and not last_seg.is_templated |
There was a problem hiding this comment.
This line is the actual fix: Ignore templated newlines when looking for rule violations.
| @@ -202,7 +206,7 @@ def _eval(self, context: RuleContext) -> Optional[LintResult]: | |||
| # no code precedes the current position: no issue | |||
| if last_seg is None: | |||
| return None | |||
| if last_seg.is_type("comma"): | |||
| if last_seg.is_type("comma") and not context.segment.is_templated: | |||
There was a problem hiding this comment.
Same fix as above, but for the leading comma case.
Brief summary of the change made
Fixes #2688, #2706
Are there any other side effects of this change that we should be aware of?
Pull Request checklist
Please confirm you have completed any of the necessary steps below.
Included test cases to demonstrate any code changes, which may be one or more of the following:
.ymlrule test cases intest/fixtures/rules/std_rule_cases..sql/.ymlparser test cases intest/fixtures/dialects(note YML files can be auto generated withtox -e generate-fixture-yml).test/fixtures/linter/autofix.Added appropriate documentation for the change.
Created GitHub issues for any relevant followup/future enhancements if appropriate.