{"action":"opened","issue":{"url":"https://api.github.com/repos/findbugsproject/findbugs/issues/127","repository_url":"https://api.github.com/repos/findbugsproject/findbugs","labels_url":"https://api.github.com/repos/findbugsproject/findbugs/issues/127/labels{/name}","comments_url":"https://api.github.com/repos/findbugsproject/findbugs/issues/127/comments","events_url":"https://api.github.com/repos/findbugsproject/findbugs/issues/127/events","html_url":"https://github.com/findbugsproject/findbugs/issues/127","id":178238934,"number":127,"title":"UserPreferences should not use short detector names","user":{"login":"jsotuyod","id":802626,"avatar_url":"https://avatars.githubusercontent.com/u/802626?v=3","gravatar_id":"","url":"https://api.github.com/users/jsotuyod","html_url":"https://github.com/jsotuyod","followers_url":"https://api.github.com/users/jsotuyod/followers","following_url":"https://api.github.com/users/jsotuyod/following{/other_user}","gists_url":"https://api.github.com/users/jsotuyod/gists{/gist_id}","starred_url":"https://api.github.com/users/jsotuyod/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/jsotuyod/subscriptions","organizations_url":"https://api.github.com/users/jsotuyod/orgs","repos_url":"https://api.github.com/users/jsotuyod/repos","events_url":"https://api.github.com/users/jsotuyod/events{/privacy}","received_events_url":"https://api.github.com/users/jsotuyod/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":0,"created_at":"2016-09-21T03:48:20Z","updated_at":"2016-09-21T03:48:20Z","closed_at":null,"body":"Recently, fb-contrib has had an issue where a detector would sometimes detect or not an issue at random (see https://github.com/mebigfatguy/fb-contrib/issues/138)\r\n\r\nThe issue seems to be rooted at how Findbugs creates detectors. It wraps each on a [`DetectorFactory`](https://github.com/findbugsproject/findbugs/blob/master/findbugs/src/java/edu/umd/cs/findbugs/DetectorFactory.java) which will create it on-demand through reflection, but before doing this. it will check if the detector is enabled. To do so, Findbugs performs the following:\r\n\r\n```java\r\n    public static boolean isDetectorEnabled(IFindBugsEngine findBugs, DetectorFactory factory, int rankThreshold) {\r\n\r\n        if (!findBugs.getUserPreferences().isDetectorEnabled(factory)) {\r\n            return false;\r\n        }\r\n```\r\n[Source](https://github.com/findbugsproject/findbugs/blob/master/findbugs/src/java/edu/umd/cs/findbugs/FindBugs.java#L250-L252)\r\n\r\n```java\r\n    public boolean isDetectorEnabled(DetectorFactory factory) {\r\n        String detectorName = factory.getShortName();\r\n        Boolean enabled = detectorEnablementMap.get(detectorName);\r\n        if (enabled == null) {\r\n            // No explicit preference has been specified for this detector,\r\n            // so use the default enablement specified by the\r\n            // DetectorFactory.\r\n            enabled = factory.isDefaultEnabled();\r\n            detectorEnablementMap.put(detectorName, enabled);\r\n        }\r\n        return enabled;\r\n    }\r\n```\r\n[Source](https://github.com/findbugsproject/findbugs/blob/master/findbugs/src/java/edu/umd/cs/findbugs/config/UserPreferences.java#L405-L416)\r\n\r\nWhere, as you can guess, `factory.getShortName()` is:\r\n\r\n```java\r\n    public String getShortName() {\r\n        int endOfPkg = className.lastIndexOf('.');\r\n        if (endOfPkg >= 0) {\r\n            return className.substring(endOfPkg + 1);\r\n        }\r\n        return className;\r\n    }\r\n```\r\n[Source](https://github.com/findbugsproject/findbugs/blob/master/findbugs/src/java/edu/umd/cs/findbugs/DetectorFactory.java#L403-L409)\r\n\r\nSo, `com.mebigfatguy.fbcontrib.detect.FindCircularDependencies` and `edu.umd.cs.findbugs.detect.FindCircularDependencies` collide in the map. Whichever gets loaded first, enables / disables both.\r\n\r\nUsing simple class names is flaky, since there is no guarantee of uniqueness (as shown!). Findbugs should either use fully qualified class names for all detectors, or at least, do so with plugins (if for some reason backwards compatibility is expected)."}}
{"actor":{"display_login":"jsotuyod"}}