Skip to content
This repository has been archived by the owner on Apr 13, 2023. It is now read-only.

maybe "assert" shouldnt always show the constraint #3725

Open
CeylonMigrationBot opened this issue Mar 2, 2013 · 13 comments
Open

maybe "assert" shouldnt always show the constraint #3725

CeylonMigrationBot opened this issue Mar 2, 2013 · 13 comments

Comments

@CeylonMigrationBot
Copy link

[@DiegoCoronel] Hi,

lets say i have this assert:

doc "Hours should be between 0 and 23"
assert( hours >= 0 && hours < h.perDay );

and i want to use the method: time( 25, 0); in this case its not a valid hour and the message i got from AssertException is:

"Assertion failed: Hours should be between 0 and 23
 violated hours >= 0 && hours < h.perDay"

so, why do we need to know the assert that caused it? isnt this internally ?

in above case isnt so bad: violated hours >= 0 && hours < h.perDay
but i think there will be cases with xxx[2] < yyy.xor(123)... some calc with no sense for the caller (developer)

isnt better show only the doc when its avaiable?

[Migrated from ceylon/ceylon-spec#619]

@CeylonMigrationBot
Copy link
Author

[@RossTate] This may also be a problem for those trying to keep their code a secret.

@CeylonMigrationBot
Copy link
Author

[@loicrouchon] I like the way assert is writing the failing condition but given your example (a clear doc annotation) I understand your point.
For now, and I can see two possible solutions:

  • If there is a doc annotation on an assert, then, only return doc content in case of violation
  • Or, add an optional parameter to assert to tell him not to return the violated condition

The second solution would maybe be more flexible than the first one, but I don't really know what we should (or not) do

@CeylonMigrationBot
Copy link
Author

[@tombentley] A third option would be a compiler option to disable inclusion of the violated condition. That would probably be preferred by those seeking secrecy because it would let them set it in one place, rather than having to remember something every time they used assert.

@CeylonMigrationBot
Copy link
Author

[@gavinking]

A third option would be a compiler option to disable inclusion of the violated condition.

This makes more sense to me. At development time, which is when we usually see assertions, it's really useful to see the expression. Once we distribute the app or library, the doc string is probably enough.

But from my point of view the biggest issue here remains that we can't do interpolation into the assertion failure message.

@CeylonMigrationBot
Copy link
Author

[@tombentley]

we can't do interpolation into the assertion failure message.

Well isn't that because we're abusing the doc annotation (where you need a String literal in order to be able to generate sensible documentation), when really we should have another way to specify the message of the AssertionException, while still allowing the assert to be documentable with doc.

The problem here was syntax, as I recall. We could use something like assert(cond1, cond2, cond3; "interpolatable"), which could be optionally preceded with a doc string. Or you could use a different annotation from doc of course, but that's not very pretty imho.

@CeylonMigrationBot
Copy link
Author

[@gavinking]
@tombentley Right, we would need a way to specify an interpolated string (or even just a string expression) as part of the assert statement. Perhaps:

assert(cond1, cond2, cond3) => "interpolatable";

Though the fat arrow would not really be necessary.

@CeylonMigrationBot
Copy link
Author

[@gavinking] Perhaps this:

assert (0<=x<max) else "x must be nonnegative and smaller than ``max``";

@CeylonMigrationBot
Copy link
Author

[@tombentley] My immediate reaction was "yeah, that's great!", followed moments later by the following doubts:

  • In if/else we require use of braces, but in assert/else we would not. Although that makes sense (once you know the else of an assert can only contain a String expression), I can imagine people learning the syntax would find it confusing.
  • assert/else is now almost the same as if/else{ throw }, except for the scope of the type narrowing.

I think I still prefer it over => though.

@CeylonMigrationBot
Copy link
Author

[@gavinking] @tombentley We already allow else <expr> as part of the expression syntax, so it's not irregular to my eyes.

@CeylonMigrationBot
Copy link
Author

[@tombentley] Sure but that's in an expression, so it makes intuitive sense. Now the rule about when else has braces isn't as simple as expression vs statement. Put another way we now have a statement level else which isn't guarding a block. I'm not saying it's a problem as such, it just seems like something that would trip up someone learning the language.

@CeylonMigrationBot
Copy link
Author

[@gavinking] Thinking through this issue a little further, I'm not convinced its a real problem. Assertions failures represent bugs, i.e. programming errors. I don't see why you would ever need to hide the expression given that the audience is always the programmer.

And I don't attach any weight on this "people trying to keep their code secret" argument. that one doesn't make a whole lot of sense.

I'm inclined to close this.

@CeylonMigrationBot
Copy link
Author

[@tombentley] The audience is not always the programmer; the bug might not be discovered until the software is shipped/deployed/whatever.

If some code makes it into production with such a bug present, a user who discovers the bug can also provoke the program into revealing a little bit of information about the source code (and something the programmer believed was true, that isn't). I that extra information could in principal help turn a plain bug into a bug with security implications.

@CeylonMigrationBot
Copy link
Author

[@gavinking] The eventual audience is the programmer. And I don't buy your scenario at all. If you are in the habit of displaying stack traces to users (I'm not), then there's already a whole lot of information there. it's not like this is a dynamic lang where perhaps the user could take advantage of knowing some local var names to do eval or something.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

1 participant