feat!: linting rule no unregistered external models (#5009)

Author: benfdking

examples/sushi/config.py

@@ -48,6 +48,7 @@
             "noselectstar",
             "nomissingaudits",
             "nomissingowner",
+            "nomissingexternalmodels",
         ],
     ),
 )

sqlmesh/core/linter/rules/builtin.py

@@ -10,7 +10,7 @@
 from sqlmesh.core.linter.helpers import TokenPositionDetails, get_range_of_model_block
 from sqlmesh.core.linter.rule import Rule, RuleViolation, Range, Fix, TextEdit
 from sqlmesh.core.linter.definition import RuleSet
-from sqlmesh.core.model import Model, SqlModel
+from sqlmesh.core.model import Model, SqlModel, ExternalModel
 
 
 class NoSelectStar(Rule):
@@ -110,4 +110,31 @@ def check_model(self, model: Model) -> t.Optional[RuleViolation]:
             return self.violation()
 
 
+class NoMissingExternalModels(Rule):
+    """All external models must be registered in the external_models.yaml file"""
+
+    def check_model(self, model: Model) -> t.Optional[RuleViolation]:
+        # Ignore external models themselves, because either they are registered,
+        # and if they are not, they will be caught as referenced in another model.
+        if isinstance(model, ExternalModel):
+            return None
+
+        # Handle other models that may refer to the external models.
+        not_registered_external_models: t.Set[str] = set()
+        for depends_on_model in model.depends_on:
+            existing_model = self.context.get_model(depends_on_model)
+            if existing_model is None:
+                not_registered_external_models.add(depends_on_model)
+
+        if not not_registered_external_models:
+            return None
+
+        return RuleViolation(
+            rule=self,
+            violation_msg=f"Model '{model.name}' depends on unregistered external models: "
+            f"{', '.join(m for m in not_registered_external_models)}. "
+            "Please register them in the external models file. This can be done by running 'sqlmesh create_external_models'.",
+        )
+
+
 BUILTIN_RULES = RuleSet(subclasses(__name__, Rule, (Rule,)))

tests/core/linter/test_builtin.py

@@ -0,0 +1,50 @@
+import os
+
+from sqlmesh import Context
+
+
+def test_no_missing_external_models(tmp_path, copy_to_temp_path) -> None:
+    """
+    Tests that the linter correctly identifies unregistered external model dependencies.
+
+    This test removes the `external_models.yaml` file from the sushi example project,
+    enables the linter, and verifies that the linter raises a violation for a model
+    that depends on unregistered external models.
+    """
+    sushi_paths = copy_to_temp_path("examples/sushi")
+    sushi_path = sushi_paths[0]
+
+    # Remove the external_models.yaml file
+    os.remove(sushi_path / "external_models.yaml")
+
+    # Override the config.py to turn on lint
+    with open(sushi_path / "config.py", "r") as f:
+        read_file = f.read()
+
+    before = """    linter=LinterConfig(
+        enabled=False,
+        rules=[
+            "ambiguousorinvalidcolumn",
+            "invalidselectstarexpansion",
+            "noselectstar",
+            "nomissingaudits",
+            "nomissingowner",
+            "nomissingexternalmodels",
+        ],
+    ),"""
+    after = """linter=LinterConfig(enabled=True, rules=["nomissingexternalmodels"]),"""
+    read_file = read_file.replace(before, after)
+    assert after in read_file
+    with open(sushi_path / "config.py", "w") as f:
+        f.writelines(read_file)
+
+    # Load the context with the temporary sushi path
+    context = Context(paths=[sushi_path])
+
+    # Lint the models
+    lints = context.lint_models(raise_on_error=False)
+    assert len(lints) == 1
+    assert (
+        "Model 'sushi.customers' depends on unregistered external models: "
+        in lints[0].violation_msg
+    )

vscode/extension/tests/quickfix.spec.ts

@@ -17,20 +17,28 @@ test('noselectstar quickfix', async ({ page, sharedCodeServer }) => {
   const configPath = path.join(tempDir, 'config.py')
   const read = await fs.readFile(configPath, 'utf8')
   // Replace linter to be on
+  const target = 'enabled=True'
   const replaced = read.replace('enabled=False', 'enabled=True')
+  // Assert replaced correctly
+  expect(replaced).toContain(target)
+
+  // Replace the rules to only have noselectstar
+  const targetRules = `rules=[
+            "noselectstar",
+        ],`
   const replacedTheOtherRules = replaced.replace(
     `rules=[
             "ambiguousorinvalidcolumn",
             "invalidselectstarexpansion",
             "noselectstar",
             "nomissingaudits",
             "nomissingowner",
+            "nomissingexternalmodels",
         ],`,
-    `rules=[
-            "noselectstar",
-        ],
-`,
+    targetRules,
   )
+  expect(replacedTheOtherRules).toContain(targetRules)
+
   await fs.writeFile(configPath, replacedTheOtherRules)
   // Replace the file to cause the error
   const modelPath = path.join(tempDir, 'models', 'latest_order.sql')