Clojure

Badly formed pre/post conditions silently passed

Details

  • Type: Enhancement Enhancement
  • Status: Open Open
  • Priority: Major Major
  • Resolution: Unresolved
  • Affects Version/s: None
  • Fix Version/s: None
  • Component/s: None
  • Labels:
  • Patch:
    Code and Test
  • Approval:
    Triaged

Description

Before:

user=> ((fn [x] {:pre (pos? x)} x) -5) ; ouch!
-5
user=> ((fn [x] {:pre [(pos? x)]} x) -5) ; meant this
AssertionError Assert failed: (pos? x)  user/eval4075/fn--4076 (form-init5464179453862723045.clj:1)

After:

user=> ((fn [x] {:pre (pos? x)} x) -5)
CompilerException java.lang.IllegalArgumentException: Pre and post conditions should be vectors, compiling:(NO_SOURCE_PATH:1:2) 
user=> ((fn [x] {:pre [(pos? x)]} x) -5)                                  
AssertionError Assert failed: (pos? x)  user/eval2/fn--3 (NO_SOURCE_FILE:2)
user=> ((fn [x] {:post (pos? x)} x) -5)
CompilerException java.lang.IllegalArgumentException: Pre and post conditions should be vectors, compiling:(NO_SOURCE_PATH:3:2) 
user=> ((fn [x] {:post [(pos? x)]} x) -5)              
AssertionError Assert failed: (pos? x)  user/eval7/fn--8 (NO_SOURCE_FILE:4)

Patch: CLJ-1473_v03.patch
Screened by: Alex Miller

Activity

Hide
Alex Miller added a comment -

Would be nice to include the bad condition in the error (maybe via ex-info?) and also have tests.

Show
Alex Miller added a comment - Would be nice to include the bad condition in the error (maybe via ex-info?) and also have tests.
Hide
Brandon Bloom added a comment -

New patch includes tests. Unfortunately, can't call ex-info directly due to bootstrapping concerns. Instead, just calls ExceptionInfo constructor directly.

Show
Brandon Bloom added a comment - New patch includes tests. Unfortunately, can't call ex-info directly due to bootstrapping concerns. Instead, just calls ExceptionInfo constructor directly.
Hide
Alex Miller added a comment -

Bug in the reporting: {:post pre} should be {:post post}.

Test should be improved as it could have caught that.

Show
Alex Miller added a comment - Bug in the reporting: {:post pre} should be {:post post}. Test should be improved as it could have caught that.
Hide
Brandon Bloom added a comment -

Good catch with the pre/post copy/paste screw up. Didn't enhance the test though, since that would involve creating an ex-info friendly variant of fails-with-cause

Show
Brandon Bloom added a comment - Good catch with the pre/post copy/paste screw up. Didn't enhance the test though, since that would involve creating an ex-info friendly variant of fails-with-cause
Hide
Rich Hickey added a comment -

:pre and :post don't require vectors, just collections

Show
Rich Hickey added a comment - :pre and :post don't require vectors, just collections
Hide
Andy Fingerhut added a comment -

Eastwood 0.2.2, released on Nov 15 2015, will warn about several kinds of incorrect pre and postconditions. See https://github.com/jonase/eastwood#wrong-pre-post

The Eastwood documentation may be misleading right now, in that it says that :pre and :post should be vectors, which is at odds with Rich's comment of Oct 9 2015. Corrections to Eastwood's documentation here are welcome. I guess Rich's intent is that :pre and :post could be vectors, lists, or sets? Would a map ever make sense there?

Show
Andy Fingerhut added a comment - Eastwood 0.2.2, released on Nov 15 2015, will warn about several kinds of incorrect pre and postconditions. See https://github.com/jonase/eastwood#wrong-pre-post The Eastwood documentation may be misleading right now, in that it says that :pre and :post should be vectors, which is at odds with Rich's comment of Oct 9 2015. Corrections to Eastwood's documentation here are welcome. I guess Rich's intent is that :pre and :post could be vectors, lists, or sets? Would a map ever make sense there?

People

Vote (4)
Watch (3)

Dates

  • Created:
    Updated: