search

sevntu-checkstyle / methods-distance

GNU Lesser General Public License v3.0
5 stars 13 forks source link

Analysis fails on abstract classes #53

Closed wilcoln closed 4 years ago

wilcoln commented 4 years ago

the command java -jar methods-distance/target/methods-distance-1.0-SNAPSHOT-jar-with-dependencies.jar --generate-dsm --generate-dot /home/wilcoln/checkstyle/src/main/java/com/puppycrawl/tools/checkstyle/api/AbstractFileSetCheck.java

Produces the following error

Starting audit...
Exception in thread "main" com.puppycrawl.tools.checkstyle.api.CheckstyleException: Exception was thrown while processing /home/wilcoln/Workspace/Projects/checkstyle/src/main/java/com/puppycrawl/tools/checkstyle/api/AbstractFileSetCheck.java
    at com.puppycrawl.tools.checkstyle.Checker.processFiles(Checker.java:304)
    at com.puppycrawl.tools.checkstyle.Checker.process(Checker.java:216)
    at com.github.sevntu.checkstyle.common.MethodCallDependencyCheckInvoker.invoke(MethodCallDependencyCheckInvoker.java:77)
    at com.github.sevntu.checkstyle.Main.main(Main.java:62)
Caused by: java.lang.NullPointerException
    at com.github.sevntu.checkstyle.analysis.MethodDefinitionParser.getLength(MethodDefinitionParser.java:265)
    at com.github.sevntu.checkstyle.analysis.MethodDefinitionParser.parse(MethodDefinitionParser.java:78)
    at com.github.sevntu.checkstyle.domain.ClassDefinition.lambda$getDeclaredMethods$0(ClassDefinition.java:62)
    at java.util.stream.ReferencePipeline$3$1.accept(ReferencePipeline.java:193)
    at java.util.ArrayList$ArrayListSpliterator.forEachRemaining(ArrayList.java:1382)
    at java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:481)
    at java.util.stream.AbstractPipeline.wrapAndCopyInto(AbstractPipeline.java:471)
    at java.util.stream.ReduceOps$ReduceOp.evaluateSequential(ReduceOps.java:708)
    at java.util.stream.AbstractPipeline.evaluate(AbstractPipeline.java:234)
    at java.util.stream.ReferencePipeline.collect(ReferencePipeline.java:499)
    at com.github.sevntu.checkstyle.domain.ClassDefinition.getDeclaredMethods(ClassDefinition.java:63)
    at com.github.sevntu.checkstyle.domain.ClassDefinition.<init>(ClassDefinition.java:47)
    at com.github.sevntu.checkstyle.module.MethodCallDependencyCheckstyleModule.buildDependencies(MethodCallDependencyCheckstyleModule.java:117)
    at com.github.sevntu.checkstyle.module.MethodCallDependencyCheckstyleModule.lambda$null$0(MethodCallDependencyCheckstyleModule.java:106)
    at java.util.Optional.ifPresent(Optional.java:159)
    at com.github.sevntu.checkstyle.module.MethodCallDependencyCheckstyleModule.lambda$finishTree$1(MethodCallDependencyCheckstyleModule.java:104)
    at java.util.Optional.ifPresent(Optional.java:159)
    at com.github.sevntu.checkstyle.module.MethodCallDependencyCheckstyleModule.finishTree(MethodCallDependencyCheckstyleModule.java:103)
    at com.puppycrawl.tools.checkstyle.TreeWalker.notifyEnd(TreeWalker.java:327)
    at com.puppycrawl.tools.checkstyle.TreeWalker.walk(TreeWalker.java:284)
    at com.puppycrawl.tools.checkstyle.TreeWalker.processFiltered(TreeWalker.java:161)
    at com.puppycrawl.tools.checkstyle.api.AbstractFileSetCheck.process(AbstractFileSetCheck.java:85)
    at com.puppycrawl.tools.checkstyle.Checker.processFile(Checker.java:329)
    at com.puppycrawl.tools.checkstyle.Checker.processFiles(Checker.java:291)
    ... 3 more

The error message makes me believe that failure is due to an attempt to parse the definition of an abstract method, this is indeed impossible, as such methods do not have any.

Code should be updated to prevent such attempts.

I think that the tool should work on abstract classes as well, since they can sometimes, contain many defined methods for the sake of factorization.

If issue is approved I am willing to work on it.

rnveach commented 4 years ago

This repo has no approve label right now but consider it approved.

https://github.com/sevntu-checkstyle/methods-distance/blob/101e94a2bf423720b655399e7d23f3430482afde/methods-distance/src/main/java/com/github/sevntu/checkstyle/analysis/MethodDefinitionParser.java#L263-L265 I agree with your analysis, the problem looks like an issue with a method that has no curlies, like a native/abstract method.

What is your recommendation on the fix? Always return a value of 0?

wilcoln commented 4 years ago

Yes that's exactly what I was thinking about. We can do that for starters and then confirm that fix makes sens by investigating DSM and penalty function value for abstract classes.