-
-
Notifications
You must be signed in to change notification settings - Fork 15.5k
Add the tool "nixos-typecheck" that can check an option declaration to: #13607
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
- Enforce that an option declaration has a "defaultText" if and only if the type of the option derives from "package", "packageSet" or "nixpkgsConfig" and if a "default" attribute is defined. - Enforce that the value of the "example" attribute is wrapped with "literalExample" if the type of the option derives from "package", "packageSet" or "nixpkgsConfig". - Warn if a "defaultText" is defined in an option declaration if the type of the option does not derive from "package", "packageSet" or "nixpkgsConfig". - Warn if no "type" is defined in an option declaration.
- Enforce that an option declaration has a "defaultText" if and only if the type of the option derives from "package", "packageSet" or "nixpkgsConfig" and if a "default" attribute is defined. - Enforce that the value of the "example" attribute is wrapped with "literalExample" if the type of the option derives from "package", "packageSet" or "nixpkgsConfig". - Warn if a "defaultText" is defined in an option declaration if the type of the option does not derive from "package", "packageSet" or "nixpkgsConfig". - Warn if no "type" is defined in an option declaration.
cc @oxij |
# A module option taking a function can not be introspected and documented properly. | ||
functionTo = resultType: | ||
mkOptionType { | ||
name = "function to ${resultType.name}"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Never again, I shall see this thing coming back to life!
Whoever need this should burn in misery under the flames of my fury!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm absolutely with you on that! The problem is that functions are currently used as options in a few modules. So my plan was to name them first, and then to deprecate the use of functions as options in a next step.
@ts468 , The suggested change is highly intrusive. I can understand the will of adding the But I do not understand what is this type inference story or how it influence in plays in the module evaluation. All values, definitions, declarations are available for introspection without the need to make such invasive changes. This is how |
then inferredType mode /* Set to 'true' to see every type that is being inferred, not just those types that result in 'unspecified'. */ | ||
else if mode != "silent" then builtins.trace noInferDoc types.unspecified else types.unspecified; | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@nbp
I assume you're referring to this function, and all the changes needed to add it, as the quite intrusive one?
Ideally, we would reject any option without an explicit type definition. This function is just there to help us getting towards explicitly defined option types by showing all the options that need to be typed.
So my plan was to add this function temporarily, and to switch the default value from 'silent' to 'printUnspecified' to 'printAll' over time, and then to replace the function with a hard error in case an untyped option is found.
I would prefer to have that enforcing mechanism within the module system directly, instead of in an external tool. And I thought this would be the right place for it. What do you think?
@nbp So what I want to achieve is to make sure that all options of the module system are permanently checked to always have a 'type'. And if an option contains a derivation, it should also be checked to use 'defaultText' and 'literalExample' if a default or example is given. So I don't know whether that approach is the best one, and I'm very open for any suggestions on how to ensure the options are fully specified. |
@ts468
And expose anything you need this way. From what I understand, making this as part of the module system adds overhead that every user will have to go through, while having an external tool add such overhead only when we are checking for correctness, such as within hydra, or with |
@nbp Could you maybe explain a little bit more on what you mean by exposing everything I need similarly like:
I'm afraid I don't see it myself. Concerning the overhead for every user, I actually hoped that the implementation would keep that quite low. |
Yes, basically nixos provide a What I suggesting, is that yoiu modify options under the This change should not be intrusive, as they will only collect information, and should require only minimal changes to modules.nix file (no more that 10 lines of changes). |
@nbp |
Thank you for your contributions. This has been automatically marked as stale because it has had no activity for 180 days. If this is still important to you, we ask that you leave a comment below. Your comment can be as simple as "still important to me". This lets people see that at least one person still cares about this. Someone will have to do this at most twice a year if there is no other activity. Here are suggestions that might help resolve this more quickly:
|
Is this still relevant? |
Add the tool "nixos-typecheck" that can check an option declaration to:
Enforce that an option declaration has a "defaultText" if and only if the
type of the option derives from "package", "packageSet" or "nixpkgsConfig"
and if a "default" attribute is defined.
Enforce that the value of the "example" attribute is wrapped with "literalExample"
if the type of the option derives from "package", "packageSet" or "nixpkgsConfig".
Warn if a "defaultText" is defined in an option declaration if the type of
the option does not derive from "package", "packageSet" or "nixpkgsConfig".
Warn if no "type" is defined in an option declaration.
Remove all errors that were found by "nixos-typecheck".