define-check: only add location if not already on stack

[bennn]

Change define-check to only add a location for the check if the current-check-info stack does not have a location on it.

(I think this is what check-true and others used to do. I'm not sure if they did the same for other check-info items too.)

With this PR,

this program fails with a good error message:

#lang racket/base
(require rackunit (for-syntax racket/base syntax/parse syntax/srcloc))

(define-syntax (check-true* stx)
  (syntax-parse stx
   [(_ . e*)
    #`(begin .
        #,(for/list ([e (in-list (syntax-e #'e*))])
            #`(with-check-info* (list (make-check-location '#,(build-source-location-list e)))
                (λ () (check-true #,e)))))]))

(check-true*
 #true
 #false)

Without this PR,

the above program needs to be rewritten something like this to get a good error message:

#lang racket/base
(require rackunit (for-syntax racket/base syntax/parse syntax/srcloc))

(define (call/updated-location loc thnk)
  (define old-around (current-check-around))
  (define (new-around check-thnk)
    (with-check-info* (list (make-check-location loc))
      (λ () (old-around check-thnk))))
  (parameterize ([current-check-around new-around])
    (thnk)))

(define-syntax (check-true* stx)
  (syntax-parse stx
    [(_ . e*)
    #`(begin .
        #,(for/list ([e (in-list (syntax-e #'e*))])
            #`(call/updated-location '#,(build-source-location-list e) (λ () (check-true #,e)))))]))

(check-true*
 #true
 #false)

---

In case we'd rather close this PR without merging, I looked in the main distribution for uses of with-check-info that updated the location. The only ones I found were in the rackunit-abbrevs and typed-racket-test collections. I can change those to do the call/updated-location dance.

[bennn]

The PR & issues about check-exn ignoring its message argument are/were related to this (#65).

I'm thinking, as a general rule, the checks in RackUnit should not add things to the check info stack if there's matching info already on the stack. (Like default arguments to a function.)

[stchang]

Many of my tests failed to report the right source location, due to a change that occurred between 6.10 and 6.11.

This pr fixes my issue.

[jackfirth]

I think this is the right thing to do and think you've got the right rule here:

I'm thinking, as a general rule, the checks in RackUnit should not add things to the check info stack if there's matching info already on the stack. (Like default arguments to a function.)

I likely won't have time to review this (or do anything else with RackUnit) this week or weekend but I'm planning on getting to it by the end of the month. If before then you'd like to make a PR that makes all "automatic" check infos added by define-check behave like this, I have nothing but encouragement.

[bennn]

I changed the docs (and code) to say that define-check adds 'location and 'expression if they aren't already there.

This is different from the rule I proposed above, but I think it makes sense based on todays reading of the documentation. I'll re-read next weekend. EDIT: that was not a good idea; in practice I think people want to be able to define new checks from existing ones, and to do that well you want to be able to override everything that a basic check will do by default

[bennn]

Updated,

• "automatic" check-info values are now only added if not already on the stack
• the docs for "Custom Checks" try to explain this
• added a new function with-default-check-info* --- like with-check-info* but does not override existing infos