Disallow declaring options named config and options (and others)

[infinisil]

Issue description

Options being allowed to be named config and options causes problems related to module syntax. The canonical module syntax has a number of sections, mainly config, options, imports, disabledModules and freeformType, which are the only allowed top-level attributes. This might look like

{
  imports = [ ... ];
  options = ...;
  config = ...;

  # Not allowed
  foo = ...;
}

But another syntax, sometimes called shorthand syntax, is also supported, which triggers when neither options nor config is set. In this alternate syntax, setting arbitrary attributes is valid, with the semantics that they get put into the config section. This might look like this:

{
  imports = [ ... ];
  # options, config <- needs to not be defined
  
  foo = ...;
}

This is then equivalent to

{
  imports = [ ... ];
  config.foo = ...;
}

If you don't need any option definitions, this syntax is more convenient.

---

The problem arises when you have options with names matching the section names, such as config, options, imports, or others. Assuming we have an option named options, then we can't use the convenient shorthand syntax:

{
  # Since `options` is declared, the canonical syntax is used, not the shorthand one,
  # making this be an option declaration, not a definition!
  options = ...;
}

To actually declare this option you need to use

{
  config.options = ...;
}

Similarly, if you have an option named config, you can't do

{
  config = ...;
}

but instead need to write

{
  config.config = ...;
}

If options have a name matching another section name, like imports or disallowedModules, shorthand syntax also can't be used, because these attributes are interpreted as sections, not options definitions. E.g. with an option named imports, the following doesn't work:

{
  imports = ...;
}

Instead this is needed:

{
  config.imports = ...;
}

---

These same problems are also caused by submodules, except that there it's even more nuanced, because submodules declared with types.submodule historically have always forced the shorthand syntax by wrapping its definitions into a config value. This was done to prevent surprises when trying to set a config option. E.g. if you have an option foo of type types.submodule, which itself has a nested config option, then you can do this:

{
  foo.config = ...;
}

Instead of having to do this:

{
  foo.config.config = ...;
}

But the flip side is that there's no way to then declare non-config sections. The only known hack around this is to declare values as a function, because only attribute values are wrapped under config (since anything else wouldn't be valid as av config value):

{
  foo = { ... }: {
    options = ...;
  };
}

With the introduction of types.submoduleWith, a parameter shorthandOnlyDefinesConfig was introduced, which controls whether this config wrapping is done, defaulting to it not being done (in contrast with types.submodule)

Suggested solution

By disallowing options to be named according to a section name, we can always know whether an attribute is a section or an option name, allowing us to avoid all these problems:

• No more weird edge cases regarding shorthand syntax: Every option can be defined using shorthand syntax.
• The whole config wrapping for types.submodule and types.submoduleWith can be removed, cleaning up code and removing the need for function hacks.
• We should also change the rule of "options or config being set implies canonical syntax" to just "config being set implies canonical syntax". If options can't be named options, then there's no reason options couldn't be declared using an options attribute while using shorthand syntax for config.

The hardest part is backwards compatibility and transition, because in NixOS there are a whole bunch of options named config and some named options (and one named imports). Another section name that could be problematic is key.

We should also consider whether it's possible to rename config and options to something else. I've heard complaints in the past that these have been confusing.

Ping @roberth @zimbatm

[zimbatm]

Sounds good, disallowing defining options that could also be in the top-level makes sense since we're allowing to collapse the config prefix.

One way to fix that is to check those during evaluation and fail if they exist. Then add yet another top-level allowTopLevelKeys = [] that as a whitelist on top of the previous check. That way it gives a path for users to upgrade. They can temporarily whitelist the keys until they rename them to something else. That logic would also apply to sub-modules. Does that make sense?

There are some other points presented, but they don't seem more adjacent than being a requirement to this core change (I would love to rename options to schema)

[mohe2015]

What about the settings RFC and options inside there that are named upstream like this? Could this cause problems?

[roberth]

What about the settings RFC and options inside there that are named upstream like this? Could this cause problems?

Yes. We should take care not to make it worse than the current situation, where the user can solve it by putting the settings in a config.

[infinisil]

What about the settings RFC and options inside there that are named upstream like this? Could this cause problems?

Oh damn yeah I didn't think of that, that makes this a bit more tricky..

So I think we can still allow arbitrary option names, but throw an ambiguity error when shorthand syntax is used with a value that could be either a section name or an option name, which then forces the canonical syntax to be used in such cases. While this makes setting options matching a section name a bit annoying (since you have to do { config.{config,options,imports,...} = ...; }, I think that's not too bad if we make sure to rename these options when possible in nixpkgs, but also require explicit confirmation that an option needs to be named like this for other cases. E.g.

{
  options.options = lib.mkOption {
    # If this is not passed, a warning or an error is thrown
    # This is only needed (and allowed) for options that match a section name
    # The effect of this option is that it requires canonical syntax in ambiguous cases 
    ignoreSectionNameConflict = true;
  };
}

This way, we can restrict such options only to where they're really needed with NixOS/rfcs#42

[zimbatm]

Sounds good. For new nixos users, this collapsing logic is not super clear. Most examples they first encounter are config-only and they get confused about the nature of nixos modules. Anything that makes this a bit stricter and exposes the underlying structure is a good thing. I would even be in favour of forcing the imports, config, ... prefixes by default unless a collapse = true; option was passed.

[nh2]

Anything that makes this a bit stricter and exposes the underlying structure is a good thing. I would even be in favour of forcing the imports, config, ... prefixes by default unless a collapse = true; option was passed.

I agree. Having multiple ways to do the same basic thing made NixOS options quite confusing to me when I started, and still sometimes tricks me today.