[rfc][merge] ban assert statements

[John Arbash Meinel]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Martin Pool wrote:
| I think we should ban assert statements from Bazaar: they cause bugs
| and don't particularly help:
|
|  * it makes the behaviour vary depending on whether bzr is run with -O
| or not, therefore giving a chance for bugs that occur in one case or
| the other, several of which have already occurred: assertions with
| side effects, code which can't continue unless the assertion passes,
| cases where we should give the user a proper message rather than an
| assertion failure
|  * it's not that much shorter than an explicit if/raise
|  * it tends to lead to fuzzy thinking about whether the check is
| actually needed or not, and whether it's an internal error or not
|  * tends to cause look-before-you-leap patterns
|  * it's unsafe if the check is needed to protect the integrity of the
| user's data
|  * it tends to give poor messages since the developer can get by with
| no explanatory text at all
|  * we can't rely on people always running with -O in normal use, so we
| can't use it for tests that are actually expensive
|  * expensive checks that help developers are better turned on from the
| test suite or a -D flag
|  * if used instead of self.assert*() in tests it makes them falsely pass with -O
|
| This patch adds a test for them; it traps about 15 files that
| currently use them.  If people agree with this in principle I'll post
| a patch that changes them to if/raise with AssertionError ValueError
| in most cases.  This will not magically make these errors more
| understandable if they occur to a user, but I wouldn't be surprised if
| I find some of the cases above.

I would certainly ban asserts without associated tests. Overall, I think assert
has probably caused more problems for us than benefits. I think the *good* of it
is that it is fast to type, and has caught some errors in our code, that not
having anything would be bad.

However, 'assert' should never be used to check data-from-outside anyway.

So, overall I'm happy enough to refuse 'assert' in source code. We can always
use it while developing, and then fix it before it gets merged into mainline.

John
=:->

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (Cygwin)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iEYEARECAAYFAkgN7xsACgkQJdeBCYSNAAMMaQCeLIvO+AzRyco6ArJh1uJ7e1Zh
3r0AnRC7TEjBl9W1M5pIC3C+7BzZ4scO
=HNiy
-----END PGP SIGNATURE-----