From 818a6810e09764a997d5b34555a711845cdeb8ac Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Anselm=20Sch=C3=BCler?= Date: Wed, 21 Sep 2022 00:01:48 +0200 Subject: [PATCH 1/6] rfc-0135: init --- rfcs/0135-custom-asserts.md | 99 +++++++++++++++++++++++++++++++++++++ 1 file changed, 99 insertions(+) create mode 100644 rfcs/0135-custom-asserts.md diff --git a/rfcs/0135-custom-asserts.md b/rfcs/0135-custom-asserts.md new file mode 100644 index 000000000..8920824fd --- /dev/null +++ b/rfcs/0135-custom-asserts.md @@ -0,0 +1,99 @@ +--- +feature: custom-asserts +start-date: 2022-09-21 +author: Anselm Schüler +co-authors: None +shepherd-team: None +shepherd-leader: None +related-issues: None +--- + +# Summary +[summary]: #summary + +Allow users to use attribute sets with a boolean attribute `success` and a string attribute `message` instead of a boolean in `assert …; …` expressions. + +# Motivation +[motivation]: #motivation + +Since Nix is an untyped language, asserts are often needed to ensure that a function or program works correctly. However, the current assert system is unsuitable for more sophisticated error reporting that aims to inform the user as to what has happened. +Consider, for instance, this simple type system: + +```nix +rec { + assertType = type: locDescr: value: + if ! type.check value + then throw "Value at ${locDescr} was not of type ${type.name}" + else builtins.traceVerbose "Successfully checked type ${type.name} at ${locDescr}" value; + assertTypeSeq = type: locDescr: value1: value2: + builtins.seq (assertType type locDescr value1) value2; + intType = { + check = builtins.isInt; + name = "integer"; + }; + attrsOfType = subType: { + check = value: + builtins.isAttrs value + && builtins.all subType.check (builtins.attrValues value); + name = "attribute set of ${subType.name}"; + }; +} +``` + +Users of this system would be forced to forgo the convenience of imperative-style `assert …; …` in favor of `seq`-like syntax in order to benefit from improved type errors. With this change, no longer! A variant `isType` function could be declared: + +```nix +{ + isType = type: locDescr: value: + let + success = type.check value; + message = if success + then "Value at ${locDescr} was not of type ${type.name}" + else "Successfully checked type ${type.name} at ${locDescr}"; + in { inherit success message; }; +} +``` + +Compare three implementations of a function that only takes attribute sets of integers: + +```nix +with types; +{ + onlyTakesAttrsOfInt1 = value: + assertType (attrsOfType intType) "onlyTakesAttrsOfInt1" value; + onlyTakesAttrsOfInt2 = value: + assertTypeSeq (attrsOfType intType) "onlyTakesAttrsOfInt1" value value; + onlyTakesAttrsOfInt3 = value: + assert isType (attrsOfType intType) "onlyTakesAttrsOfInt1" value; + value; +} +``` + +While these implementations get more and more verbose, they also get more and more idiomatic and flexible. + +# Detailed design +[design]: #detailed-design + +Allow users to use attribute sets with a boolean attribute `success` and a string attribute `message` instead of a boolean in `assert …; …` expressions. +If the `success` attribute is false, the assertion fails with a message including the `message` attribute. +If `--trace-verbose` is enabled, and the assertion succeeds, a message including the `message` attribute is also printed. + +This change requires no change to the language grammar. + +# Drawbacks +[drawbacks]: #drawbacks + +- Implementation clutter +- This could end up as a confusing and underutilized feature that hardly anybody knows about + +# Alternatives +[alternatives]: #alternatives + +- Doing nothing and continuing to use whichever assertion method is most appropriate for a given use case +- Deprecating `assert …; …` expressions in favour of user-made type systems + +# Unresolved questions +[questions]: #unresolved-questions + +- Should `message` on successful assertions be printed if `--trace-verbose` is passed, or is that too verbose? +- What should the exact name of `message` and `success` be? From 95f82867a6cd22989701627ad6d657ce97b25b94 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Anselm=20Sch=C3=BCler?= Date: Wed, 21 Sep 2022 11:36:09 +0200 Subject: [PATCH 2/6] rfc-0135: add better reasoning --- rfcs/0135-custom-asserts.md | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/rfcs/0135-custom-asserts.md b/rfcs/0135-custom-asserts.md index 8920824fd..fe832868c 100644 --- a/rfcs/0135-custom-asserts.md +++ b/rfcs/0135-custom-asserts.md @@ -71,12 +71,28 @@ with types; While these implementations get more and more verbose, they also get more and more idiomatic and flexible. +Consider also a nixpkgs package expression that wants to validate its arguments. Currently, the best way to +provide a custom error message is to use `assert … -> throw …; …`. +This method has several disadvantages: Since an implication from a falsehood is always true, you are required +to invert the condition. Additionally, since the assertion itself is not triggered by the error, +the function of the `assert` keyword is reduced to providing an imperative shorthand for `seq`. This also means that by default, +the error location is not printed, and there is no mention of an assert in the error message. +Instead, expressions could now use this more natural syntax: + +```nix +{ foo, bar }: +assert { + success = foo || bar; + message = "At least one of Foo or Bar must be set"; +}; +[ foo bar ] +``` + # Detailed design [design]: #detailed-design Allow users to use attribute sets with a boolean attribute `success` and a string attribute `message` instead of a boolean in `assert …; …` expressions. If the `success` attribute is false, the assertion fails with a message including the `message` attribute. -If `--trace-verbose` is enabled, and the assertion succeeds, a message including the `message` attribute is also printed. This change requires no change to the language grammar. @@ -95,5 +111,4 @@ This change requires no change to the language grammar. # Unresolved questions [questions]: #unresolved-questions -- Should `message` on successful assertions be printed if `--trace-verbose` is passed, or is that too verbose? - What should the exact name of `message` and `success` be? From 4a117163ce37255ee5870a97fb1d85b333ebea60 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Anselm=20Sch=C3=BCler?= Date: Wed, 21 Sep 2022 11:38:18 +0200 Subject: [PATCH 3/6] rfc-0135: add 1 question --- rfcs/0135-custom-asserts.md | 1 + 1 file changed, 1 insertion(+) diff --git a/rfcs/0135-custom-asserts.md b/rfcs/0135-custom-asserts.md index fe832868c..5f3eb3718 100644 --- a/rfcs/0135-custom-asserts.md +++ b/rfcs/0135-custom-asserts.md @@ -112,3 +112,4 @@ This change requires no change to the language grammar. [questions]: #unresolved-questions - What should the exact name of `message` and `success` be? +- What should the error messsage be? From ad0b9ed46a3ab693bf2d87700e784498048f835e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Anselm=20Sch=C3=BCler?= Date: Wed, 21 Sep 2022 12:05:30 +0200 Subject: [PATCH 4/6] rfc-0135: reorder & spelling --- rfcs/0135-custom-asserts.md | 37 +++++++++++++++++++------------------ 1 file changed, 19 insertions(+), 18 deletions(-) diff --git a/rfcs/0135-custom-asserts.md b/rfcs/0135-custom-asserts.md index 5f3eb3718..af4617be9 100644 --- a/rfcs/0135-custom-asserts.md +++ b/rfcs/0135-custom-asserts.md @@ -17,7 +17,25 @@ Allow users to use attribute sets with a boolean attribute `success` and a strin [motivation]: #motivation Since Nix is an untyped language, asserts are often needed to ensure that a function or program works correctly. However, the current assert system is unsuitable for more sophisticated error reporting that aims to inform the user as to what has happened. -Consider, for instance, this simple type system: + +Consider a nixpkgs package expression that wants to validate its arguments. Currently, the best way to +provide a custom error message is to use `assert … -> throw …; …`. +This method has several disadvantages: Since an implication from a falsehood is always true, you are required +to invert the condition. Additionally, since the assertion itself is not triggered by the error, +the function of the `assert` keyword is reduced to providing an imperative shorthand for `seq`. This also means that by default, +the error location is not printed, and there is no mention of an assert in the error message. +Instead, expressions could use this more natural syntax: + +```nix +{ foo, bar }: +assert { + success = foo || bar; + message = "At least one of foo or bar must be set"; +}; +[ foo bar ] +``` + +Also consider, for instance, this simple type system: ```nix rec { @@ -71,23 +89,6 @@ with types; While these implementations get more and more verbose, they also get more and more idiomatic and flexible. -Consider also a nixpkgs package expression that wants to validate its arguments. Currently, the best way to -provide a custom error message is to use `assert … -> throw …; …`. -This method has several disadvantages: Since an implication from a falsehood is always true, you are required -to invert the condition. Additionally, since the assertion itself is not triggered by the error, -the function of the `assert` keyword is reduced to providing an imperative shorthand for `seq`. This also means that by default, -the error location is not printed, and there is no mention of an assert in the error message. -Instead, expressions could now use this more natural syntax: - -```nix -{ foo, bar }: -assert { - success = foo || bar; - message = "At least one of Foo or Bar must be set"; -}; -[ foo bar ] -``` - # Detailed design [design]: #detailed-design From 7a5c67e65aab6907fda188380f890837929dc552 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Anselm=20Sch=C3=BCler?= Date: Wed, 21 Sep 2022 15:25:14 +0200 Subject: [PATCH 5/6] rfc-0135: remove note on inversion disadvantage --- rfcs/0135-custom-asserts.md | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/rfcs/0135-custom-asserts.md b/rfcs/0135-custom-asserts.md index af4617be9..a37b0f1e9 100644 --- a/rfcs/0135-custom-asserts.md +++ b/rfcs/0135-custom-asserts.md @@ -19,9 +19,8 @@ Allow users to use attribute sets with a boolean attribute `success` and a strin Since Nix is an untyped language, asserts are often needed to ensure that a function or program works correctly. However, the current assert system is unsuitable for more sophisticated error reporting that aims to inform the user as to what has happened. Consider a nixpkgs package expression that wants to validate its arguments. Currently, the best way to -provide a custom error message is to use `assert … -> throw …; …`. -This method has several disadvantages: Since an implication from a falsehood is always true, you are required -to invert the condition. Additionally, since the assertion itself is not triggered by the error, +provide a custom error message is to use `assert … || throw …; …`. +This method has several disadvantages: Since the assertion itself is not triggered by the error, the function of the `assert` keyword is reduced to providing an imperative shorthand for `seq`. This also means that by default, the error location is not printed, and there is no mention of an assert in the error message. Instead, expressions could use this more natural syntax: From 955f2e9503911b581913a402fd19c82de57e8daf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Anselm=20Sch=C3=BCler?= Date: Wed, 21 Sep 2022 15:26:37 +0200 Subject: [PATCH 6/6] rfc-0135: remove incorrect statement --- rfcs/0135-custom-asserts.md | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/rfcs/0135-custom-asserts.md b/rfcs/0135-custom-asserts.md index a37b0f1e9..728fa8970 100644 --- a/rfcs/0135-custom-asserts.md +++ b/rfcs/0135-custom-asserts.md @@ -21,8 +21,7 @@ Since Nix is an untyped language, asserts are often needed to ensure that a func Consider a nixpkgs package expression that wants to validate its arguments. Currently, the best way to provide a custom error message is to use `assert … || throw …; …`. This method has several disadvantages: Since the assertion itself is not triggered by the error, -the function of the `assert` keyword is reduced to providing an imperative shorthand for `seq`. This also means that by default, -the error location is not printed, and there is no mention of an assert in the error message. +the function of the `assert` keyword is reduced to providing an imperative shorthand for `seq`. Instead, expressions could use this more natural syntax: ```nix