Exclude self from violations of onlyAccessedBy(..) by default #3
Comments
|
Actually this might be slightly harder, than I thought, since when checking for "onlyAccessedBy", it's not clear anymore, how the classes were filtered before (could be reside in package, but could also be annotated, named, or whatever), so it's hard to determine what "self" is. The class itself is no problem, but the package I chose on |
…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.)
…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.)
|
Is there a possibility at least the "self" part is revisited? Here is what i try to : services
.should().bePackagePrivate()
.andShould().onlyBeAccessed().byItself()//doesnt exists
.because("Service implementations should never be accessed directly"); |
…e three try catch blocks start from a handler label.
At least with my JDK 7 it looks like this:
```
private void method(int, boolean);
descriptor: (IZ)V
flags: (0x0002) ACC_PRIVATE
Code:
stack=4, locals=6, args_size=3
0: new TNG#3 // class java/net/Socket
3: dup
4: ldc TNG#4 // String
6: iconst_0
7: invokespecial TNG#5 // Method java/net/Socket."<init>":(Ljava/lang/String;I)V
10: astore_3
11: new TNG#6 // class java/io/BufferedReader
14: dup
15: aconst_null
16: invokespecial TNG#7 // Method java/io/BufferedReader."<init>":(Ljava/io/Reader;)V
19: astore 4
21: aload 4
23: invokevirtual TNG#8 // Method java/io/BufferedReader.close:()V
26: return
27: astore 5
29: aload 4
31: invokevirtual TNG#8 // Method java/io/BufferedReader.close:()V
34: aload 5
36: athrow
37: astore_3
38: return
Exception table:
from to target type
27 29 27 any
0 26 37 Class java/lang/Exception
27 37 37 Class java/lang/Exception
```
We can possibly ignore this, since we will drop JDK 7 support soon anyway, but let's maybe ponder about it, why it looks so different. That of the two declared try-catch-blocks only one try catch block is visible is definitely strange (and worse than having 3 try-catch-blocks where one is synthetic). But this example is anyway quite strange, because the bytecode immediately changes e.g. if we remove one of the unused method parameters.
Signed-off-by: Peter Gafert <peter.gafert@tngtech.com>
|
Hey @codecholeric thanks for your work! I saw you fixed the initial issue with commit fbca4c9 which is packaged in Expected behavior is tested by proposed change So basically I'm trying to pick all classes from a
Would you have any understanding about that please? I wonder if the fix is not covering this case, or if I'm using the conjuction in a bad manner? Thank you! |
|
Unfortunately, the fix fbca4c9 you mentioned was reverted in #233. So currently you actually would have to do |
When checking
it would make sense to ignore violations by self access by default. Often usecases look like
because otherwise a ton of violations of '..bar..' calling itself would be reported.
It seems to never make sense, to report self-accesses, so this should be excluded by default, making rules easier to specify.
The text was updated successfully, but these errors were encountered: