add new SLF4J_GET_STACK_TRACE detector (fixes #70) #72

vorburger · 2018-03-14T16:09:59Z

vorburger · 2018-03-14T16:12:06Z

@KengoTODA this fails locally for me with this message, which makes no sense to me, can you help:

-------------------------------------------------------------------------------
Test set: jp.skypencil.findbugs.slf4j.UsingGetStackTraceTest
-------------------------------------------------------------------------------
Tests run: 1, Failures: 0, Errors: 1, Skipped: 0, Time elapsed: 0.079 sec <<< FAILURE! - in jp.skypencil.findbugs.slf4j.UsingGetStackTraceTest
testExceptionMethods()  Time elapsed: 0.076 sec  <<< ERROR!
java.lang.ExceptionInInitializerError
	at jp.skypencil.findbugs.slf4j.UsingGetStackTraceTest.testExceptionMethods(UsingGetStackTraceTest.java:18)
Caused by: java.lang.IllegalStateException: Unable to load core plugin
	at jp.skypencil.findbugs.slf4j.UsingGetStackTraceTest.testExceptionMethods(UsingGetStackTraceTest.java:18)
Caused by: edu.umd.cs.findbugs.PluginDoesntContainMetadataException: Core pluginfindbugs-2.0.2.jar doesn't contain findbugs.xml; got jar:file:/home/vorburger/dev/github/KengoTODA/findbugs-slf4j/bug-pattern/target/bug-pattern-1.4.1-SNAPSHOT.jar!/findbugs.xml from sun.misc.Launcher$AppClassLoader[file:/home/vorburger/dev/github/KengoTODA/findbugs-slf4j/test-case/target/surefire/surefirebooter1722547952841405469.jar]
	at jp.skypencil.findbugs.slf4j.UsingGetStackTraceTest.testExceptionMethods(UsingGetStackTraceTest.java:18)

That bug-pattern-1.4.1-SNAPSHOT.jar DOES contain findbugs.xml, so not sure what it's trying to tell me... 😃

vorburger · 2018-03-14T16:20:34Z

@KengoTODA I've written the test differently in the 3rd commit (like the existing ones), so now it passes.

KengoTODA

Update CHANGELOG.md and README.md.

I will read implementation changes carefully, later. At least I don't like naming like AbstractDetectorForParameterArray2.

vorburger · 2018-03-16T15:13:49Z

Update CHANGELOG.md and README.md.

done.

I will read implementation changes carefully, later.

thanks!

At least I don't like naming like AbstractDetectorForParameterArray2.

how would you name it?

vorburger · 2018-04-03T14:15:54Z

@KengoTODA how is this coming along - any chance we can wrap this up soon-ish?

KengoTODA · 2018-05-28T09:26:03Z

bug-pattern/src/main/java/jp/skypencil/findbugs/slf4j/ManualGetStackTraceDetector.java

+    }
+
+    @Override
+    protected int getIsOfInterestKind() {


please annotate this method with @Item.SpecialKind

KengoTODA · 2018-05-28T09:26:43Z

bug-pattern/src/main/java/jp/skypencil/findbugs/slf4j/ManualGetStackTraceDetector.java

+public class ManualGetStackTraceDetector extends AbstractDetectorForParameterArray2 {
+
+    @Item.SpecialKind
+    private final int isGetStackTrace = Item.defineNewSpecialKind("use of throwable getStackTrace");


isXxx is common naming for boolean, better to rename...

KengoTODA · 2018-05-28T09:28:32Z

bug-pattern/src/main/java/jp/skypencil/findbugs/slf4j/ManualGetStackTraceDetector.java

+
+    @Override
+    protected boolean isWhatWeWantToDetect(int seen) {
+        // return false;


delete needless comment

KengoTODA · 2018-05-28T09:29:37Z

.../src/main/java/jp/skypencil/findbugs/slf4j/parameter/AbstractDetectorForParameterArray2.java

+
+    abstract protected boolean isWhatWeWantToDetect(int seen);
+
+    abstract protected int getIsOfInterestKind();


getIsOf sounds strange, please rename...

KengoTODA · 2018-05-28T09:30:19Z

.../src/main/java/jp/skypencil/findbugs/slf4j/parameter/AbstractDetectorForParameterArray2.java

+import javax.annotation.Nullable;
+import jp.skypencil.findbugs.slf4j.parameter.ArrayDataHandler.Strategy;
+
+public abstract class AbstractDetectorForParameterArray2 extends AbstractDetectorForParameterArray {


here we still have bad naming

how would you name it?

vorburger · 2018-05-31T09:47:55Z

@KengoTODA I've taken all your previous code review feedback (thank you) into account. OK for you now?

vorburger · 2018-07-04T13:38:50Z

@KengoTODA would you be willing to finally merge this if I resolved the conflicts? (Those appeared after.)

vorburger · 2018-07-23T19:06:58Z

@KengoTODA I've rebased and resolved conflicts and squased this now - how about finally merging?

KengoTODA · 2018-07-24T02:29:41Z

@vorburger could you rebase your branch? I've updated master to skip SonarQube analysis for external PR.

vorburger · 2018-07-24T08:14:45Z

@vorburger could you rebase your branch?

done

vorburger · 2018-07-27T11:31:17Z

@KengoTODA I had taken all your previous code review feedback into account. How about merging?

vorburger · 2018-09-04T20:44:05Z

@KengoTODA ?

vorburger · 2018-10-10T10:32:50Z

@KengoTODA are you willing to merge this if I resolve conflicts now? I am asking because I have done that twice in the past, and can't keep spending time to rebase a PR I'm unclear about whether you even consider merging it, and have thus ignored this.

vorburger · 2019-02-05T12:49:45Z

Thought about this again when I found a catch & printStackTrace ... see https://issues.apache.org/jira/browse/FINERACT-696 ... @KengoTODA will you merge this if I resolve conflicts now? I am asking because I have done that twice in the past, and can't keep spending time to rebase a PR I'm unclear about whether you even consider merging it...

so that in the next commit the upcoming ManualGetStackTraceDetector for a SLF4J_GET_STACK_TRACE (issue KengoTODA#70) does not have to copy/paste 3/4 of the ManualMessageDetector

incl. adjustments for code review feedback * delete useless inline comment * add @Item.SpecialKind annotation * rename getIsOfInterestKind to getKindOfInterest * rename isGetStackTrace to getStackTraceKind * rename AbstractDetectorForParameterArray2 to AbstractExtendedDetectorForParameterArray

KengoTODA

If you need to change existing classes (ManualMessageDetector.java), could you separate PR into two parts: 1. changing existing classes and 2. adding new detector based on it? It will help me to focus on purpose of review.

vorburger · 2021-12-04T10:00:08Z

Hello @KengoTODA I'm doing a year end clean up of my personal https://github.com/pulls and wanted to ask if you would still consider merging this very old Pull Request if I rebased it, or if we should forget about and close this? (This is a bulk message I'm posting to many issues to gauge what is still revelant.)

KengoTODA requested changes Mar 16, 2018

View reviewed changes

vorburger mentioned this pull request Apr 3, 2018

SLF4J_MANUALLY_PROVIDED_MESSAGE should be able to fail even if only getMessage() and no cause #69

Open

KengoTODA requested changes May 28, 2018

View reviewed changes

vorburger force-pushed the issue-70_getStackTrace branch from 65fae45 to 7c819fa Compare July 23, 2018 19:06

vorburger mentioned this pull request Jul 23, 2018

TravisCI build sonar-maven-plugin failure due to: Failed to connect to localhost/127.0.0.1:9000: Connection refused (Connection refused) #111

Closed

vorburger force-pushed the issue-70_getStackTrace branch from 7c819fa to b0d7d80 Compare July 24, 2018 08:14

vorburger mentioned this pull request Aug 6, 2018

RFE: New SLF4J_LOST_EXCEPTION #74

Open

vorburger force-pushed the issue-70_getStackTrace branch from 6eedefb to 287cdfd Compare February 5, 2019 12:58

vorburger added 2 commits February 5, 2019 12:59

factor AbstractDetectorForParameterArray2 out of ManualMessageDetector

cfa372b

so that in the next commit the upcoming ManualGetStackTraceDetector for a SLF4J_GET_STACK_TRACE (issue KengoTODA#70) does not have to copy/paste 3/4 of the ManualMessageDetector

vorburger force-pushed the issue-70_getStackTrace branch 2 times, most recently from 2801d1a to 0841ad7 Compare February 5, 2019 13:06

vorburger mentioned this pull request Feb 5, 2019

add .factorypath to .gitignore #179

Merged

KengoTODA requested changes Feb 5, 2019

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add new SLF4J_GET_STACK_TRACE detector (fixes #70) #72

add new SLF4J_GET_STACK_TRACE detector (fixes #70) #72

vorburger commented Mar 14, 2018 •

edited

Loading

vorburger commented Mar 14, 2018

vorburger commented Mar 14, 2018

KengoTODA left a comment •

edited

Loading

vorburger commented Mar 16, 2018

vorburger commented Apr 3, 2018

KengoTODA May 28, 2018

KengoTODA May 28, 2018

vorburger May 30, 2018

KengoTODA May 28, 2018

vorburger May 30, 2018

KengoTODA May 28, 2018

vorburger May 30, 2018

KengoTODA May 28, 2018

vorburger May 30, 2018

vorburger May 30, 2018

vorburger commented May 31, 2018

vorburger commented Jul 4, 2018

vorburger commented Jul 23, 2018

KengoTODA commented Jul 24, 2018

vorburger commented Jul 24, 2018

vorburger commented Jul 27, 2018

vorburger commented Sep 4, 2018

vorburger commented Oct 10, 2018

vorburger commented Feb 5, 2019

KengoTODA left a comment

vorburger commented Dec 4, 2021


		abstract protected boolean isWhatWeWantToDetect(int seen);

		abstract protected int getIsOfInterestKind();

add new SLF4J_GET_STACK_TRACE detector (fixes #70) #72

Are you sure you want to change the base?

add new SLF4J_GET_STACK_TRACE detector (fixes #70) #72

Conversation

vorburger commented Mar 14, 2018 • edited Loading

vorburger commented Mar 14, 2018

vorburger commented Mar 14, 2018

KengoTODA left a comment • edited Loading

Choose a reason for hiding this comment

vorburger commented Mar 16, 2018

vorburger commented Apr 3, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vorburger commented May 31, 2018

vorburger commented Jul 4, 2018

vorburger commented Jul 23, 2018

KengoTODA commented Jul 24, 2018

vorburger commented Jul 24, 2018

vorburger commented Jul 27, 2018

vorburger commented Sep 4, 2018

vorburger commented Oct 10, 2018

vorburger commented Feb 5, 2019

KengoTODA left a comment

Choose a reason for hiding this comment

vorburger commented Dec 4, 2021

vorburger commented Mar 14, 2018 •

edited

Loading

KengoTODA left a comment •

edited

Loading