Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions docs/examples/sqlmesh_cli_crash_course.md
Original file line number Diff line number Diff line change
Expand Up @@ -675,6 +675,8 @@ This is a great way to catch SQL issues before wasting runtime in your data ware

```bash
sqlmesh lint
# or apply fixes automatically
sqlmesh lint --fix
```

=== "Tobiko Cloud"
Expand Down
2 changes: 1 addition & 1 deletion docs/guides/linter.md
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ Place a rule's code in the project's `linter/` directory. SQLMesh will load all

If the rule is specified in the project's [configuration file](#applying-linting-rules), SQLMesh will run it when:
- A plan is created during `sqlmesh plan`
- The command `sqlmesh lint` is ran
- The command `sqlmesh lint` is ran. Add `--fix` to automatically apply available fixes and fail if errors remain.

SQLMesh will error if a model violates the rule, informing you which model(s) violated the rule. In this example, `full_model.sql` violated the `NoMissingOwner` rule, essentially halting execution:

Expand Down
1 change: 1 addition & 0 deletions docs/reference/cli.md
Original file line number Diff line number Diff line change
Expand Up @@ -636,6 +636,7 @@ Usage: sqlmesh lint [OPTIONS]

Options:
--model TEXT A model to lint. Multiple models can be linted. If no models are specified, every model will be linted.
--fix Apply fixes for lint errors. Fails if errors remain after fixes are applied.
Copy link

@simon-pactum simon-pactum Aug 19, 2025

Choose a reason for hiding this comment

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

Would be nice to fix and still exit non-zero, makes it easier to use the same script to both fix things locally and error in CI, without needing an extra layer of pre-commit or something to detect if files were changed.

Similar discussion over at ruff, where they ended up introducing --exit-non-zero-on-fix astral-sh/ruff#8191

Just my 2c as a user - this isn't a review

--help Show this message and exit.

```
8 changes: 7 additions & 1 deletion sqlmesh/cli/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -1201,15 +1201,21 @@ def environments(obj: Context) -> None:
multiple=True,
help="A model to lint. Multiple models can be linted. If no models are specified, every model will be linted.",
)
@click.option(
"--fix",
is_flag=True,
help="Apply fixes for lint errors. Fails if errors remain after fixes are applied.",
)
@click.pass_obj
@error_handler
@cli_analytics
def lint(
obj: Context,
models: t.Iterator[str],
fix: bool,
) -> None:
"""Run the linter for the target model(s)."""
obj.lint_models(models)
obj.lint_models(models, fix=fix)


@cli.group(no_args_is_help=True)
Expand Down
34 changes: 34 additions & 0 deletions sqlmesh/core/context.py
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,7 @@
from sqlmesh.core.environment import Environment, EnvironmentNamingInfo, EnvironmentStatements
from sqlmesh.core.loader import Loader
from sqlmesh.core.linter.definition import AnnotatedRuleViolation, Linter
from sqlmesh.core.linter.rule import TextEdit, Position
from sqlmesh.core.linter.rules import BUILTIN_RULES
from sqlmesh.core.macros import ExecutableOrMacro, macro
from sqlmesh.core.metric import Metric, rewrite
Expand Down Expand Up @@ -3099,6 +3100,7 @@ def lint_models(
self,
models: t.Optional[t.Iterable[t.Union[str, Model]]] = None,
raise_on_error: bool = True,
fix: bool = False,
) -> t.List[AnnotatedRuleViolation]:
found_error = False

Expand All @@ -3116,13 +3118,45 @@ def lint_models(
found_error = True
all_violations.extend(violations)

if fix:
self._apply_fixes(all_violations)
self.refresh()
return self.lint_models(models, raise_on_error=raise_on_error, fix=False)

if raise_on_error and found_error:
raise LinterError(
"Linter detected errors in the code. Please fix them before proceeding."
)

return all_violations

def _apply_fixes(self, violations: t.List[AnnotatedRuleViolation]) -> None:
edits_by_file: t.Dict[Path, t.List[TextEdit]] = {}
for violation in violations:
for fix in violation.fixes:
for create in fix.create_files:
create.path.parent.mkdir(parents=True, exist_ok=True)
create.path.write_text(create.text, encoding="utf-8")
for edit in fix.edits:
edits_by_file.setdefault(edit.path, []).append(edit)
Copy link
Contributor

Choose a reason for hiding this comment

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

Can there be conflicting edits? How do we handle them?


for path, edits in edits_by_file.items():
content = path.read_text(encoding="utf-8")
lines = content.splitlines(keepends=True)

def _offset(pos: Position) -> int:
return sum(len(lines[i]) for i in range(pos.line)) + pos.character

for edit in sorted(
edits, key=lambda e: (e.range.start.line, e.range.start.character), reverse=True
):
start = _offset(edit.range.start)
end = _offset(edit.range.end)
content = content[:start] + edit.new_text + content[end:]
lines = content.splitlines(keepends=True)

path.write_text(content, encoding="utf-8")

def load_model_tests(
self, tests: t.Optional[t.List[str]] = None, patterns: list[str] | None = None
) -> t.List[ModelTestMetadata]:
Expand Down
55 changes: 55 additions & 0 deletions tests/cli/test_cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -1328,6 +1328,61 @@ def test_lint(runner, tmp_path):
assert result.exit_code == 1


def test_lint_fix(runner, tmp_path):
create_example_project(tmp_path)

with open(tmp_path / "config.yaml", "a", encoding="utf-8") as f:
f.write(
"""linter:
enabled: True
rules: ["noselectstar"]
"""
)

model_path = tmp_path / "models" / "incremental_model.sql"
with open(model_path, "r", encoding="utf-8") as f:
content = f.read()
content = content.replace(
"SELECT\n id,\n item_id,\n event_date,\nFROM",
"SELECT *\nFROM",
Copy link
Preview

Copilot AI Aug 11, 2025

Choose a reason for hiding this comment

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

The string replacement pattern is fragile and depends on exact whitespace matching. Consider using a more robust approach like regex replacement or a more specific pattern that's less likely to break with formatting changes.

Suggested change
"SELECT *\nFROM",
content = re.sub(
r"SELECT\s*id,\s*item_id,\s*event_date,\s*FROM",
"SELECT *\nFROM",
content,
flags=re.MULTILINE,

Copilot uses AI. Check for mistakes.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think this is a valid comment. Can we just overwrite the file, or even better, create a new one to avoid assuming a certain structure?

)
with open(model_path, "w", encoding="utf-8") as f:
f.write(content)

result = runner.invoke(cli, ["--paths", tmp_path, "lint", "--fix"])
assert result.exit_code == 0
with open(model_path, "r", encoding="utf-8") as f:
assert "SELECT *" not in f.read()
Comment on lines +1354 to +1355
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this check for the exact expected output instead of this negation?



def test_lint_fix_unfixable_error(runner, tmp_path):
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we notify users when a lint error is unfixable?

create_example_project(tmp_path)

with open(tmp_path / "config.yaml", "a", encoding="utf-8") as f:
f.write(
"""linter:
enabled: True
rules: ["noselectstar", "nomissingaudits"]
"""
)

model_path = tmp_path / "models" / "incremental_model.sql"
with open(model_path, "r", encoding="utf-8") as f:
content = f.read()
content = content.replace(
"SELECT\n id,\n item_id,\n event_date,\nFROM",
"SELECT *\nFROM",
)
with open(model_path, "w", encoding="utf-8") as f:
f.write(content)
Copy link
Preview

Copilot AI Aug 11, 2025

Choose a reason for hiding this comment

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

This string replacement pattern is duplicated from the previous test. Consider extracting this into a helper function to reduce code duplication and improve maintainability.

Suggested change
f.write(content)
replace_select_columns_with_star(model_path)

Copilot uses AI. Check for mistakes.


result = runner.invoke(cli, ["--paths", tmp_path, "lint", "--fix"])
assert result.exit_code == 1
assert "nomissingaudits" in result.output
with open(model_path, "r", encoding="utf-8") as f:
assert "SELECT *" not in f.read()


def test_state_export(runner: CliRunner, tmp_path: Path) -> None:
create_example_project(tmp_path)

Expand Down