using class binary name same as CmdLineProcessor

Review Request #1489 — Created Dec. 15, 2014 and submitted

peiyu
pants
868
d4b3568...
pants-reviews
benjyw, dturner-tw, ity, jinfeng, jsirois, patricklaw, zundel

CmdLineProcessor uses binary class name (http://docs.oracle.com/javase/specs/jvms/se7/html/jvms-4.html#jvms-4.2.1),

https://github.com/twitter/commons/blob/master/src/java/com/twitter/common/args/apt/CmdLineProcessor.java#L653

Classes that uses CmdLineProcessor as annotation processor would fail,

"Expected classname -> path, got "com.twitter.common.args.ArgsTest$App -> ..."

This patch adds $ to the class name matching, and changed the format of compile/jvm/java/classes/deprecation_report.txt slightly so it's easier to load in testing

https://travis-ci.org/pantsbuild/pants/builds/44126276 passed

DT
  1. Ship It!

  2. 
      
JI
  1. Ship It!

  2. 
      
IT
  1. please add eric, patrick, benjy, john to this too. its a good idea to have one rep from each org to make sure you get eyes from them.

    1. I appreciate the courtesy, but I'm not going to be able to comment on every review. If you tag my name and are waiting on my approval to move forward, don't hesistate to ping me.

  2. 
      
PE
ZU
  1. Ship It!

  2. 
      
DT
  1. Submitted to master.

  2. 
      
PE
Review request changed

Status: Closed (submitted)

NH
  1. Will this only be used for binary names that only come from java sources that use alphanumeric type names? Java's definition of identifier is wider than Python's, eg あ is a valid Java identifier, but not a valid Python identifier.

    The spec doesn't offer much guidance beyond saying that they must be valid unicode, and the names unqualified identifiers must not contain . ; [ /.

    http://docs.oracle.com/javase/specs/jvms/se7/html/jvms-4.html#jvms-4.2.1

    I think this change covers most class names, but doesn't cover all valid Java class names.

  2. 
      
Loading...