Reject files if they use unit by mistake

CHANGELOG.md

@@ -3,6 +3,7 @@
 ## Unreleased
 
 - Support global file-level tags in metrics.yaml ([bug 1745283](https://bugzilla.mozilla.org/show_bug.cgi?id=1745283))
+- Glinter: Reject metric files if they use `unit` by mistake. It should be `time_unit` ([#432](https://github.com/mozilla/glean_parser/pull/432)).
 
 ## 4.3.1
 

glean_parser/schemas/metrics.2-0-0.schema.yaml

@@ -434,10 +434,14 @@ definitions:
       unit:
         title: Unit
         description: |
-          The unit of the metric, for metrics that don't already require a
-          meaningful unit, such as `time_unit`.
+          The unit of the metric.
+          This is only required for metrics
+          that don't already require a meaningful unit, e.g. `quantity`
           This is provided for informational purposes only and doesn't have any
           effect on data collection.
+
+          Metric types like `timespan`, `datetime`
+          and `timing_distribution` take a `time_unit` instead.
         type: string
 
       no_lint:
@@ -693,3 +697,27 @@ additionalProperties:
                         Built-in pings are not allowed."
                       pattern:
                         "^(metrics|baseline|events|deletion-request|default|glean_.*)$"
+
+      -
+        if:
+          # This is a schema check:
+          # This is true when the checked YAML passes the schema validation.
+          #
+          # If it has a datetime/timing_distribution/timespan type
+          # AND has a `unit` property, then...
+          properties:
+            type:
+              enum:
+                - datetime
+                - timing_distribution
+                - timespan
+          required:
+            - unit
+        # ... then `time_unit` is required,
+        # because that's the only way we can force this to fail.
+        then:
+          required:
+            - time_unit
+          description: |
+            This metric type uses the (optional) `time_unit` parameter,
+            not `unit`.

tests/data/wrong_key.yamlx

@@ -0,0 +1,69 @@
+# Any copyright is dedicated to the Public Domain.
+# https://creativecommons.org/publicdomain/zero/1.0/
+
+# Note: we're using YAML anchors to re-use the values
+# defined in the first metric.
+# Saves us some typing.
+---
+$schema: moz://mozilla.org/schemas/glean/metrics/2-0-0
+
+wrong_key:
+  datetime: &unit_defaults
+    type: datetime
+    # note: this _should_ have been time_unit
+    unit: day
+    lifetime: ping
+    description: for testing
+    bugs:
+      - https://bugzilla.mozilla.org/1137353
+    data_reviews:
+      - http://example.com/
+    notification_emails:
+      - CHANGE-ME@example.com
+    expires: never
+
+  timing_distribution:
+    <<: *unit_defaults
+    type: timing_distribution
+
+  timespan:
+    <<: *unit_defaults
+    type: timespan
+
+both_keys:
+  datetime:
+    type: datetime
+    # note: it has both keys
+    unit: day
+    time_unit: day
+    lifetime: ping
+    description: for testing
+    bugs:
+      - https://bugzilla.mozilla.org/1137353
+    data_reviews:
+      - http://example.com/
+    notification_emails:
+      - CHANGE-ME@example.com
+    expires: never
+
+missing_key:
+  datetime: &defaults
+    type: datetime
+    # note: no unit specified. the `time_unit` is _optional_
+    lifetime: ping
+    description: for testing
+    bugs:
+      - https://bugzilla.mozilla.org/1137353
+    data_reviews:
+      - http://example.com/
+    notification_emails:
+      - CHANGE-ME@example.com
+    expires: never
+
+  timing_distribution:
+    <<: *defaults
+    type: timing_distribution
+
+  timespan:
+    <<: *defaults
+    type: timespan

tests/test_cli.py

@@ -155,3 +155,19 @@ def test_reject_jwe(tmpdir):
     )
     assert result.exit_code == 1
     assert len(os.listdir(str(tmpdir))) == 0
+
+
+def test_wrong_key_lint(tmpdir):
+    """Test that the 'glinter' reports a wrong key used."""
+    runner = CliRunner()
+    result = runner.invoke(
+        __main__.main,
+        [
+            "glinter",
+            str(ROOT / "data" / "wrong_key.yamlx"),
+        ],
+    )
+    assert result.exit_code == 1
+    # wrong `unit` key for datetime, timing_distribution, timespan.
+    # a missing key is NOT an error.
+    assert "Found 3 errors" in result.output