Use first line number from LineNumberTable for JavaCodeUnit's sourceCodeLocation#344
Conversation
|
👉 View analysis in DeepCode’s Dashboard |
|
Nice, thanks a lot 😃 |
|
I don't think there's a way to get what you'd consider exact... Consider this example: The auto-generated If we had instead used it would of course be in line 18... 🤷♂️ I would anyways refrain from playing any "line number – 1" trick, but just stick to the data that is actually there (to me, the only canonical options seem to be either the first or the last line number of the statements in the byte code), because
to be written as
|
|
Okay, you have convinced me 😉 I agree, we can't solve it perfectly, and an educated guess close to the declaration is still a lot nicer than line number 0 which will definitely make you search for the declaration (in any non-trivial class). And yes, keeping it simple and directly providing the info from the bytecode seems to be the way to go. Too bad we have no estimate whatsoever for fields |
codecholeric
left a comment
There was a problem hiding this comment.
lgtm 😄, I have one small comment. Also I feel we should squash these two commits. Because adapting the tests feels to me like it should be part of the same commit. WDYT?
Fair enough. (I just kept them separate as I wasn't sure whether we should make the existing failure report tests somehow ignore the line number of methods [which actually didn't work out easily] rather than encode the line number in the tests... 😉) |
1cfbd8a to
c6c0f38
Compare
…for JavaCodeUnit's sourceCodeLocation Signed-off-by: Manfred Hanke <Manfred.Hanke@tngtech.com>
c6c0f38 to
61e140f
Compare
…odeLocation #344 So far, the `sourceCodeLocation` of `JavaMember`s does not contain a line number, as it cannot reliably be inferred from the byte code (cf. #75). However, if the class file contains a [`LineNumberTable`](https://docs.oracle.com/javase/specs/jvms/se7/html/jvms-4.html#jvms-4.7.12) for a method, using the smallest line number from the `LineNumberTable` for the member's `sourceCodeLocation` – even if this probably corresponds to the first executable statement and not to the method header definition – seems to be more useful to me than using the fallback line number `0` in any case. So for example, I would infer the line number `10` from the following byte code: ``` void methodWithBodyStartingInLine10(); descriptor: ()V flags: (0x0000) Code: stack=2, locals=1, args_size=1 0: getstatic #2 // Field java/lang/System.out:Ljava/io/PrintStream; 3: bipush 10 5: invokevirtual #3 // Method java/io/PrintStream.println:(I)V 8: getstatic #2 // Field java/lang/System.out:Ljava/io/PrintStream; 11: bipush 11 13: invokevirtual #3 // Method java/io/PrintStream.println:(I)V 16: getstatic #2 // Field java/lang/System.out:Ljava/io/PrintStream; 19: bipush 12 21: invokevirtual #3 // Method java/io/PrintStream.println:(I)V 24: return LineNumberTable: line 10: 0 line 11: 8 line 12: 16 line 13: 24 ``` (My example's method header is actually defined in line 9, but I prefer 10 as opposed to 0... 😉) Note that I do even get a line number for an empty method from the following byte code: ``` void emptyMethodDefinedInLine15(); descriptor: ()V flags: (0x0000) Code: stack=0, locals=1, args_size=1 0: return LineNumberTable: line 15: 0 ``` Even if line numbers inferred in this way do not exactly point to the method header definition, they can be used to compare different methods (i.e., the ordering should be correct). This would allow for new rules like, for example (irrespective of whether I'd personally want to have them as arch rules or not): * Public methods are defined _before_ private methods. * `equals`, `hashCode` and `toString` are not generated by a framework (or a developer... 🙈) in the _same_ line (cf. #337) With this pull request, line numbers are recorded for `JavaCodeUnit`s (`JavaMethod`s, `JavaConstructor`s, `JavaStaticInitializer`s) if the class file has a `LineNumberTable`. (The reported line number `0` for `JavaField`s is unchanged, as it cannot be inferred from byte code.)
So far, the
sourceCodeLocationofJavaMembers does not contain a line number, as it cannot reliably be inferred from the byte code (cf. #75).However, if the class file contains a
LineNumberTablefor a method, using the smallest line number from theLineNumberTablefor the member'ssourceCodeLocation– even if this probably corresponds to the first executable statement and not to the method header definition – seems to be more useful to me than using the fallback line number0in any case.So for example, I would infer the line number
10from the following byte code:(My example's method header is actually defined in line 9, but I prefer 10 as opposed to 0... 😉)
Note that I do even get a line number for an empty method from the following byte code:
Even if line numbers inferred in this way do not exactly point to the method header definition, they can be used to compare different methods (i.e., the ordering should be correct). This would allow for new rules like, for example (irrespective of whether I'd personally want to have them as arch rules or not):
equals,hashCodeandtoStringare not generated by a framework (or a developer... 🙈) in the same line (cf. Arch Unit configuration with lombok [Error] #337)With this pull request, line numbers are recorded for
JavaCodeUnits (JavaMethods,JavaConstructors,JavaStaticInitializers) if the class file has aLineNumberTable.(The reported line number
0forJavaFields is unchanged, as it cannot be inferred from byte code.)