Clojure

make use of deprecated namespaces/vars easier to spot

Details

  • Type: Enhancement Enhancement
  • Status: Open Open
  • Priority: Minor Minor
  • Resolution: Unresolved
  • Affects Version/s: None
  • Fix Version/s: None
  • Component/s: None
  • Labels:
  • Patch:
    Code

Description

From the mailing list http://groups.google.com/group/clojure/msg/c41d909bd58e4534. It is easy to use deprecated namespaces without knowing you are doing so. The documentation warnings are small, and there is no compiler warning.

Some possibilities include:

  1. much more visible deprecation styling in the documentation
  2. stderr warnings when referencing a deprecated thing.

I don't love the idea of stderr warnings on all the time. Rich: is there an approach to this that you would like to see a patch for?

  1. 706-warning-on-deprecated-ns.diff
    29/Oct/12 1:02 AM
    0.9 kB
    Ghadi Shayban
  2. 706-fix-deprecation-warning-test-junit.diff
    26/Oct/12 1:37 PM
    3 kB
    Luke VanderHart
  3. 706-fix-deprecation-warnings-on-replicate.diff
    26/Oct/12 1:37 PM
    3 kB
    Luke VanderHart
  4. 706-fix-deprecation-warnings-agents.diff
    26/Oct/12 1:37 PM
    1 kB
    Luke VanderHart
  5. 706-deprecated-var-warning-patch-v2.txt
    13/Feb/13 12:38 AM
    6 kB
    Andy Fingerhut
  6. 706-deprecated-var-warning.diff
    26/Oct/12 1:37 PM
    6 kB
    Luke VanderHart

Activity

Hide
Rich Hickey added a comment -

I don't mind warning to stderr

Show
Rich Hickey added a comment - I don't mind warning to stderr
Hide
Luke VanderHart added a comment -

706-deprecated-var-warning.diff adds warnings when using deprecated vars. The other three patches clean up deprecation warnings.

Show
Luke VanderHart added a comment - 706-deprecated-var-warning.diff adds warnings when using deprecated vars. The other three patches clean up deprecation warnings.
Hide
Andy Fingerhut added a comment -

Great stuff. I looked through the first patch, and didn't see anything in there that lets someone disable deprecation warnings from the command line, the way that warn-on-reflection can today be set to true with a command line option.

Is that something important to have for deprecation warnings?

Show
Andy Fingerhut added a comment - Great stuff. I looked through the first patch, and didn't see anything in there that lets someone disable deprecation warnings from the command line, the way that warn-on-reflection can today be set to true with a command line option. Is that something important to have for deprecation warnings?
Hide
Andy Fingerhut added a comment -

I was hoping it would be quick and easy to add source file, line, and column info to the deprecation warning messages. It isn't as easy as adding them to the format() call, because the method analyzeSymbol doesn't receive these values as args. Is this deprecation check being done in a place where it is not easy to relate it to the source file, line, and column? Could it be done in a place where that info is easily available?

Show
Andy Fingerhut added a comment - I was hoping it would be quick and easy to add source file, line, and column info to the deprecation warning messages. It isn't as easy as adding them to the format() call, because the method analyzeSymbol doesn't receive these values as args. Is this deprecation check being done in a place where it is not easy to relate it to the source file, line, and column? Could it be done in a place where that info is easily available?
Hide
Ghadi Shayban added a comment -

Another patch - this time to warn on loading deprecated namespaces, instead of vars. This patch requires the first one.

Re: line/column, I'll figure out how to thread the compile context through if it's desired.

Re: Compile flag. I have a patch for this also, but I'm still verifying how to invoke. How is warn-on-reflection set by command line?

Show
Ghadi Shayban added a comment - Another patch - this time to warn on loading deprecated namespaces, instead of vars. This patch requires the first one. Re: line/column, I'll figure out how to thread the compile context through if it's desired. Re: Compile flag. I have a patch for this also, but I'm still verifying how to invoke. How is warn-on-reflection set by command line?
Hide
Andy Fingerhut added a comment -

Re: Compile flag. Don't hold off on implementing a flag if you want to, but it might be worth hearing from others whether such a command line option is even desired. I was asking in hopes of eliciting such a response.

For the way that it is handled in the Clojure compiler, search for REFLECTION_WARNING_PROP and related code in Compile.java. If you invoke the Clojure compiler directly via a Java command line, use -Dclojure.compile.warn-on-reflection=true (default is false). See the recent email thread sent to Clojure Dev Google group if you want to know how to do it via ant or Maven. Link: https://mail.google.com/mail/?shva=1#label/clojure-dev/13aa0e34530196c3

There is also a separate command-line flag called compiler-options (see Compile.java) that is implemented as a map inside the compiler. It was added more recently than warn-on-reflection, and might be the preferred method to add more such options, to avoid having to continue to add more arguments to the pushThreadBindings calls done in several places.

Show
Andy Fingerhut added a comment - Re: Compile flag. Don't hold off on implementing a flag if you want to, but it might be worth hearing from others whether such a command line option is even desired. I was asking in hopes of eliciting such a response. For the way that it is handled in the Clojure compiler, search for REFLECTION_WARNING_PROP and related code in Compile.java. If you invoke the Clojure compiler directly via a Java command line, use -Dclojure.compile.warn-on-reflection=true (default is false). See the recent email thread sent to Clojure Dev Google group if you want to know how to do it via ant or Maven. Link: https://mail.google.com/mail/?shva=1#label/clojure-dev/13aa0e34530196c3 There is also a separate command-line flag called compiler-options (see Compile.java) that is implemented as a map inside the compiler. It was added more recently than warn-on-reflection, and might be the preferred method to add more such options, to avoid having to continue to add more arguments to the pushThreadBindings calls done in several places.
Hide
Ghadi Shayban added a comment -

Thanks, Andy.

Alternately for the last ns patch, it is equivalent to call (print-method msg err), rather than binding out to err, may be more readable. I'll be glad to send that in if it's preferable.

Show
Ghadi Shayban added a comment - Thanks, Andy. Alternately for the last ns patch, it is equivalent to call (print-method msg err), rather than binding out to err, may be more readable. I'll be glad to send that in if it's preferable.
Hide
Andy Fingerhut added a comment -

706-deprecated-var-warning-patch-v2.txt dated Feb 12 2013 is identical to 706-deprecated-var-warning.diff dated Oct 26 2012, except it applies cleanly to latest master.

Show
Andy Fingerhut added a comment - 706-deprecated-var-warning-patch-v2.txt dated Feb 12 2013 is identical to 706-deprecated-var-warning.diff dated Oct 26 2012, except it applies cleanly to latest master.

People

Vote (2)
Watch (2)

Dates

  • Created:
    Updated: