Bug 2053531 - Add nimbus-fml lint

CHANGELOG.md

@@ -1,11 +1,5 @@
 # v156.0 (In progress)
 
-## ✨ What's Changed ✨
-
-### Nimbus
-
-- A new API has been added to get the list of enrolled experiments and rollouts without instantiating a NimbusClient: `get_active_enrollments()`. ([#7560](https://github.com/mozilla/application-services/pull/7560))
-
 [Full Changelog](In progress)
 
 ## ✨ What's Changed ✨
@@ -14,6 +8,14 @@
 
 - Add `LoginStore::list_candidates()` and `LoginStore::get_many()`, a pair of read APIs for consumers which filter logins on their unencrypted fields. `list_candidates()` returns a `LoginCandidate` per stored login - everything `Login` has except the secure fields (`username`/`password`), so searching by `origin`, `httpRealm` or `formActionOrigin` no longer forces a primary password prompt. `get_many()` then decrypts just the logins which matched. `list()` is unchanged, for callers who really do want every login in cleartext.
 
+### Nimbus
+
+- A new API has been added to get the list of enrolled experiments and rollouts without instantiating a NimbusClient: `get_active_enrollments()`. ([#7560](https://github.com/mozilla/application-services/pull/7560))
+
+### Nimbus FML
+
+- Add `nimbus-fml lint`, which checks a manifest against feature design lints covering metadata, descriptions, naming, and feature shape. Findings are warnings and don't affect code generation; `--error-on-warning` fails the run, for CI. A `no-lint` list on a feature or at the top of a manifest excuses it from the lints it names, so older versions of `nimbus-fml` will reject a manifest that uses one. `nimbus-fml validate` no longer reports feature metadata warnings; run `nimbus-fml lint` for those. ([Bug 2053531](https://bugzilla.mozilla.org/show_bug.cgi?id=2053531))
+
 # v155.0 (_2026-08-13_)
 
 [Full Changelog](https://github.com/mozilla/application-services/compare/v154.0...v155.0)

components/support/nimbus-fml/fixtures/fe/lints/included.fml.yaml

@@ -0,0 +1,19 @@
+---
+# Included by including.fml.yaml; excuses what it defines from a lint.
+about:
+  description: Fixture for the lint tests.
+no-lint:
+  - MISSING_META_BUG
+features:
+  included-feature:
+    description: A feature defined in a file that another file includes.
+    contacts:
+      - jdoe@example.com
+    documentation:
+      - name: User documentation
+        url: https://example.com/included-feature
+    variables:
+      enabled:
+        description: Whether the included feature does anything at all.
+        type: Boolean
+        default: false

components/support/nimbus-fml/fixtures/fe/lints/including.fml.yaml

@@ -0,0 +1,22 @@
+---
+# Includes a file that carries its own top level `no-lint` list.
+about:
+  description: Fixture for the lint tests.
+channels:
+  - release
+includes:
+  - included.fml.yaml
+features:
+  including-feature:
+    description: A feature defined in the file that does the including.
+    contacts:
+      - jdoe@example.com
+    documentation:
+      - name: User documentation
+        url: https://example.com/including-feature
+    meta-bug: https://example.com/bugs
+    variables:
+      enabled:
+        description: Whether the including feature does anything at all.
+        type: Boolean
+        default: false

components/support/nimbus-fml/fixtures/fe/lints/needs-work.fml.yaml

@@ -0,0 +1,61 @@
+---
+# Trips as many lints as fit into one file.
+about:
+  description: Fixture for the lint tests.
+channels:
+  - release
+features:
+  myBadFeature:
+    description: Bad
+    variables:
+      hide-toolbar:
+        description: TODO
+        type: Boolean
+        default: false
+      myBadFeature-mode:
+        description: Which of the layouts the feature uses.
+        type: String
+        default: compact
+      section-list:
+        description: The sections shown to the user, in the order they appear.
+        type: List<String>
+        default: []
+      deep:
+        description: A value that has to be written out four levels deep.
+        type: Outer
+        default: {}
+objects:
+  Outer:
+    description: The outermost object.
+    fields:
+      middle:
+        description: The object in the middle.
+        type: Middle
+        default: {}
+  Middle:
+    description: The object in the middle.
+    fields:
+      inner:
+        description: The innermost object.
+        type: Inner
+        default: {}
+  Inner:
+    description: The innermost object.
+    fields:
+      label:
+        description: The label shown to the user.
+        type: String
+        default: ""
+  unusedObject:
+    description: An object that no feature refers to.
+    fields:
+      label:
+        description: The label shown to the user.
+        type: String
+        default: ""
+enums:
+  OnlyOne:
+    description: An enum that doesn't offer a choice.
+    variants:
+      onlyVariant:
+        description: The only variant there is.

components/support/nimbus-fml/fixtures/fe/lints/suppressions.fml.yaml

@@ -0,0 +1,25 @@
+---
+# Excuses itself from some lints, at both levels.
+about:
+  description: Fixture for the lint tests.
+channels:
+  - release
+no-lint:
+  - MISSING_META_BUG
+  - NOT_A_REAL_FILE_LINT
+features:
+  legacy-feature:
+    description: A feature that was designed before there were lints to nudge it.
+    no-lint:
+      - MISSING_ENABLED_VARIABLE
+      - NOT_A_REAL_LINT
+    contacts:
+      - jdoe@example.com
+    documentation:
+      - name: User documentation
+        url: https://example.com/legacy-feature
+    variables:
+      max-rows:
+        description: The largest number of rows the list is allowed to grow to.
+        type: Int
+        default: 3

components/support/nimbus-fml/fixtures/fe/lints/well-formed.fml.yaml

@@ -0,0 +1,32 @@
+---
+# Trips no lints.
+about:
+  description: Fixture for the lint tests.
+channels:
+  - release
+features:
+  toolbar-redesign:
+    description: The redesigned toolbar shown at the bottom of the browser screen.
+    meta-bug: https://bugzilla.mozilla.org/show_bug.cgi?id=2053531
+    contacts:
+      - jdoe@example.com
+    documentation:
+      - name: User documentation
+        url: https://example.com/toolbar-redesign
+    variables:
+      enabled:
+        description: Whether the redesigned toolbar is shown instead of the old one.
+        type: Boolean
+        default: false
+      button-style:
+        description: How the buttons in the toolbar are drawn.
+        type: ButtonStyle
+        default: outline
+enums:
+  ButtonStyle:
+    description: The ways a toolbar button can be drawn.
+    variants:
+      outline:
+        description: The button is drawn as an outline only.
+      filled:
+        description: The button is drawn filled with the accent colour.

components/support/nimbus-fml/src/backends/frontend_manifest.rs

@@ -27,6 +27,7 @@ impl From<FeatureManifest> for ManifestFrontEnd {
             channels,
             includes: Default::default(),
             imports: Default::default(),
+            no_lint: Default::default(),
             features,
             legacy_types: None,
             types: Types { enums, objects },

components/support/nimbus-fml/src/command_line/cli.rs

@@ -33,6 +33,9 @@ pub enum Command {
     /// Validate an FML configuration and all of its channels.
     Validate(Validate),
 
+    /// Check an FML configuration against the Nimbus feature design lints.
+    Lint(Lint),
+
     /// Print out all the channels to stdout, as JSON or one-per-line
     Channels(Channels),
 
@@ -119,6 +122,40 @@ pub struct Validate {
     pub loader_info: LoaderInfo,
 }
 
+#[derive(Args)]
+pub struct Lint {
+    /// Sets the input file to use
+    #[arg(value_name = "INPUT", required_unless_present = "list")]
+    pub input: Option<String>,
+
+    #[command(flatten)]
+    pub loader_info: LoaderInfo,
+
+    /// Switch a lint off for this run. May be repeated.
+    #[arg(long, value_name = "LINT")]
+    pub allow: Vec<String>,
+
+    /// Turn a lint into an error for this run. May be repeated.
+    #[arg(long, value_name = "LINT")]
+    pub deny: Vec<String>,
+
+    /// Exit with an error if there are any warnings.
+    #[arg(long)]
+    pub error_on_warning: bool,
+
+    /// Also lint the features of imported manifests.
+    #[arg(long)]
+    pub include_imports: bool,
+
+    /// If present, then print the findings as JSON.
+    #[arg(long)]
+    pub json: bool,
+
+    /// Print the available lints and exit.
+    #[arg(long)]
+    pub list: bool,
+}
+
 #[derive(Args)]
 pub struct Channels {
     /// Sets the input file to use

components/support/nimbus-fml/src/command_line/commands.rs

@@ -14,6 +14,8 @@ pub(crate) enum CliCmd {
     GenerateSingleFileManifest(GenerateSingleFileManifestCmd),
     FetchFile(LoaderConfig, String),
     Validate(ValidateCmd),
+    Lint(LintCmd),
+    ListLints,
     PrintChannels(PrintChannelsCmd),
     PrintInfo(PrintInfoCmd),
 }
@@ -48,6 +50,16 @@ pub(crate) struct ValidateCmd {
     pub(crate) loader: LoaderConfig,
 }
 
+pub(crate) struct LintCmd {
+    pub(crate) manifest: String,
+    pub(crate) loader: LoaderConfig,
+    pub(crate) allow: Vec<String>,
+    pub(crate) deny: Vec<String>,
+    pub(crate) error_on_warning: bool,
+    pub(crate) include_imports: bool,
+    pub(crate) as_json: bool,
+}
+
 pub(crate) struct PrintChannelsCmd {
     pub(crate) manifest: String,
     pub(crate) loader: LoaderConfig,

components/support/nimbus-fml/src/command_line/mod.rs

@@ -12,7 +12,7 @@ use anyhow::Result;
 use clap::Parser;
 use commands::{
     CliCmd, GenerateExperimenterManifestCmd, GenerateSingleFileManifestCmd, GenerateStructCmd,
-    PrintChannelsCmd, ValidateCmd,
+    LintCmd, PrintChannelsCmd, ValidateCmd,
 };
 
 use std::{collections::BTreeMap, ffi::OsString, path::Path};
@@ -39,6 +39,8 @@ fn process_command(cmd: &CliCmd) -> Result<()> {
         }
         CliCmd::FetchFile(files, nm) => workflows::fetch_file(files, nm)?,
         CliCmd::Validate(params) => workflows::validate(params)?,
+        CliCmd::Lint(params) => workflows::lint(params)?,
+        CliCmd::ListLints => workflows::list_lints()?,
         CliCmd::PrintChannels(params) => workflows::print_channels(params)?,
         CliCmd::PrintInfo(params) => workflows::print_info(params)?,
     };
@@ -68,6 +70,7 @@ where
         cli::Command::Validate(cmd) => {
             CliCmd::Validate(create_validate_command_from_cli(&cmd, cwd)?)
         }
+        cli::Command::Lint(cmd) => create_lint_command_from_cli(&cmd, cwd)?,
         cli::Command::Channels(cmd) => {
             CliCmd::PrintChannels(create_print_channels_from_cli(&cmd, cwd)?)
         }
@@ -176,6 +179,26 @@ fn create_validate_command_from_cli(cmd: &cli::Validate, cwd: &Path) -> Result<V
     Ok(ValidateCmd { manifest, loader })
 }
 
+fn create_lint_command_from_cli(cmd: &cli::Lint, cwd: &Path) -> Result<CliCmd> {
+    if cmd.list {
+        return Ok(CliCmd::ListLints);
+    }
+
+    // clap has already checked that there is an input file if we're not listing.
+    let manifest = cmd.input.clone().unwrap_or_default();
+    let loader = create_loader(&manifest, &cmd.loader_info, cwd)?;
+
+    Ok(CliCmd::Lint(LintCmd {
+        manifest,
+        loader,
+        allow: cmd.allow.clone(),
+        deny: cmd.deny.clone(),
+        error_on_warning: cmd.error_on_warning,
+        include_imports: cmd.include_imports,
+        as_json: cmd.json,
+    }))
+}
+
 fn create_print_channels_from_cli(cmd: &cli::Channels, cwd: &Path) -> Result<PrintChannelsCmd> {
     let manifest = cmd.input.clone();
     let loader = create_loader(&cmd.input, &cmd.loader_info, cwd)?;