Allon Mureinik has uploaded a new change for review. Change subject: findbugs: Make inner classes static where possible ......................................................................
findbugs: Make inner classes static where possible This patch makes all inner classes static, where possible[1]. The main advantage of making an inner class static is the load removed from the classloader. This way, an inner class is created per enclosing CLASS instead of per enclosing INSTANCE. This patch also enables a FindBugs check to ensure that such mistakes are not introduced after this patch. [1] A notable exception to this rule are inner classes in classes that are GWT-compiled. Any inner class with the @GenEvent annotation is converted to a top-level class with the same modifiers by GWT, and therefore can't be declared static. Change-Id: Ifbceb1baad596211580c1289f49f7d12e33a8ecb Signed-off-by: Allon Mureinik <[email protected]> --- M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/lock/InMemoryLockManager.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/MacPoolManager.java M backend/manager/modules/extensions-manager/src/main/java/org/ovirt/engine/core/extensions/mgr/ExtensionsManager.java M backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/customprop/VmPropertiesUtils.java M backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/xmlrpc/XmlRpcUtils.java M backend/manager/tools/src/main/java/org/ovirt/engine/core/notifier/transport/smtp/Smtp.java M exclude-filters-general.xml M frontend/webadmin/modules/gwt-common/exclude-filters.xml M frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/utils/ConsoleOptionsFrontendPersisterImpl.java M frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/widget/refresh/BaseRefreshPanel.java M frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/Configurator.java M frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/events/TaskListModel.java M frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/hosts/AddClusterRM.java M frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/hosts/AddDataCenterRM.java M frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/hosts/AddStorageDomainRM.java M frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/hosts/ChangeHostClusterRM.java M frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/hosts/ConfigureLocalStorageModel.java M frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/users/UserEventNotifierListModel.java M frontend/webadmin/modules/userportal-gwtp/exclude-filters.xml M frontend/webadmin/modules/userportal-gwtp/src/main/java/org/ovirt/engine/ui/userportal/widget/resources/VmTable.java M frontend/webadmin/modules/webadmin/exclude-filters.xml 21 files changed, 44 insertions(+), 30 deletions(-) git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/27/26227/1 diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/lock/InMemoryLockManager.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/lock/InMemoryLockManager.java index 4059302..a2a4612 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/lock/InMemoryLockManager.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/lock/InMemoryLockManager.java @@ -306,7 +306,7 @@ /** * The following class represents different locks which are kept inside InMemoryLockManager */ - private class InternalLockView { + private static class InternalLockView { /** Number for shared locks **/ private int count; diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/MacPoolManager.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/MacPoolManager.java index 007ad45..c8a311a 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/MacPoolManager.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/MacPoolManager.java @@ -300,7 +300,7 @@ } @SuppressWarnings("serial") - private class MacPoolExceededMaxException extends RuntimeException { + private static class MacPoolExceededMaxException extends RuntimeException { } private boolean allowDuplicate() { diff --git a/backend/manager/modules/extensions-manager/src/main/java/org/ovirt/engine/core/extensions/mgr/ExtensionsManager.java b/backend/manager/modules/extensions-manager/src/main/java/org/ovirt/engine/core/extensions/mgr/ExtensionsManager.java index 3816bb9..61ec66a 100644 --- a/backend/manager/modules/extensions-manager/src/main/java/org/ovirt/engine/core/extensions/mgr/ExtensionsManager.java +++ b/backend/manager/modules/extensions-manager/src/main/java/org/ovirt/engine/core/extensions/mgr/ExtensionsManager.java @@ -37,7 +37,7 @@ public static final String SENSITIVE_KEYS = "ovirt.engine.extension.sensitiveKeys"; private static final String ENGINE_EXTENSION_ENABLED = "ENGINE_EXTENSION_ENABLED_"; - public class ExtensionEntry { + public static class ExtensionEntry { private File file; private boolean enabled; private boolean activated; diff --git a/backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/customprop/VmPropertiesUtils.java b/backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/customprop/VmPropertiesUtils.java index 6b6c07b..26ac04b 100644 --- a/backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/customprop/VmPropertiesUtils.java +++ b/backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/customprop/VmPropertiesUtils.java @@ -76,7 +76,7 @@ return Config.<String> getValue(ConfigValues.PredefinedVMProperties, version.getValue()); } - public class VMCustomProperties { + public static class VMCustomProperties { private final String predefinedProperties; private final String userDefinedProperties; diff --git a/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/xmlrpc/XmlRpcUtils.java b/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/xmlrpc/XmlRpcUtils.java index c70348a..efc6b90 100644 --- a/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/xmlrpc/XmlRpcUtils.java +++ b/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/xmlrpc/XmlRpcUtils.java @@ -221,7 +221,7 @@ return new InternalCallable(obj, m, args, correlationId); } - private final class InternalCallable implements Callable<Object> { + private static final class InternalCallable implements Callable<Object> { private Object obj; private Method m; diff --git a/backend/manager/tools/src/main/java/org/ovirt/engine/core/notifier/transport/smtp/Smtp.java b/backend/manager/tools/src/main/java/org/ovirt/engine/core/notifier/transport/smtp/Smtp.java index d346356..8c12ab0 100644 --- a/backend/manager/tools/src/main/java/org/ovirt/engine/core/notifier/transport/smtp/Smtp.java +++ b/backend/manager/tools/src/main/java/org/ovirt/engine/core/notifier/transport/smtp/Smtp.java @@ -126,7 +126,7 @@ } if (StringUtils.isNotEmpty(emailPassword)) { session = Session.getDefaultInstance(mailSessionProps, - new EmailAuthenticator(emailUser, emailPassword)); + new EmailAuthenticator(emailUser, emailPassword)); } else { session = Session.getInstance(mailSessionProps); } @@ -238,7 +238,7 @@ /** * An implementation of the {@link Authenticator}, holds the authentication credentials for a network connection. */ - private class EmailAuthenticator extends Authenticator { + private static class EmailAuthenticator extends Authenticator { private String userName; private String password; public EmailAuthenticator(String userName, String password) { @@ -252,7 +252,7 @@ } } - private class DispatchAttempt { + private static class DispatchAttempt { public final AuditLogEvent event; public final String address; public int retries = 0; diff --git a/exclude-filters-general.xml b/exclude-filters-general.xml index 1899916..783bb7b 100644 --- a/exclude-filters-general.xml +++ b/exclude-filters-general.xml @@ -56,15 +56,6 @@ <!-- Currently Ignoring. findbugs reason: - SIC: Should be a static inner class (SIC_INNER_SHOULD_BE_STATIC) - --> - <Match> - <Bug code="SIC" /> - </Match> - - <!-- - Currently Ignoring. - findbugs reason: ST: Write to static field from instance method (ST_WRITE_TO_STATIC_FROM_INSTANCE_METHOD) --> <Match> diff --git a/frontend/webadmin/modules/gwt-common/exclude-filters.xml b/frontend/webadmin/modules/gwt-common/exclude-filters.xml index a9af902..5b42bcf 100644 --- a/frontend/webadmin/modules/gwt-common/exclude-filters.xml +++ b/frontend/webadmin/modules/gwt-common/exclude-filters.xml @@ -168,4 +168,12 @@ <Match> <Bug pattern="BC_UNCONFIRMED_CAST"/> </Match> + + <!-- + Ignore non-static inner classes. + This pattern is used by GWT-generated code, which we have no control of + --> + <Match> + <Bug pattern="SIC_INNER_SHOULD_BE_STATIC" /> + </Match> </FindBugsFilter> diff --git a/frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/utils/ConsoleOptionsFrontendPersisterImpl.java b/frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/utils/ConsoleOptionsFrontendPersisterImpl.java index 0e72c40..83a038d 100644 --- a/frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/utils/ConsoleOptionsFrontendPersisterImpl.java +++ b/frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/utils/ConsoleOptionsFrontendPersisterImpl.java @@ -222,7 +222,7 @@ clientStorage.setLocalItem(key, Boolean.toString(value)); } - class KeyMaker { + static class KeyMaker { private final String id; private final boolean isPool; diff --git a/frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/widget/refresh/BaseRefreshPanel.java b/frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/widget/refresh/BaseRefreshPanel.java index cc699c3..ed4a3fb 100644 --- a/frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/widget/refresh/BaseRefreshPanel.java +++ b/frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/widget/refresh/BaseRefreshPanel.java @@ -128,7 +128,7 @@ /** * A custom menu bar for 'RefreshRateOptionMenuItem' items. */ - private class RefreshRateOptionsMenu extends MenuBar { + private static class RefreshRateOptionsMenu extends MenuBar { public RefreshRateOptionsMenu(boolean vertical) { super(vertical); } diff --git a/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/Configurator.java b/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/Configurator.java index 11680b2..c964971 100644 --- a/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/Configurator.java +++ b/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/Configurator.java @@ -246,7 +246,7 @@ return wsConfig != null && !"".equals(wsConfig) && !"Off".equalsIgnoreCase(wsConfig); //$NON-NLS-1$ $NON-NLS-2$ } - public final class FileFetchEventArgs extends EventArgs { + public static final class FileFetchEventArgs extends EventArgs { private String fileContent; public String getFileContent() { diff --git a/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/events/TaskListModel.java b/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/events/TaskListModel.java index cd88279..f557ee1 100644 --- a/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/events/TaskListModel.java +++ b/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/events/TaskListModel.java @@ -267,7 +267,7 @@ // detailTaskGuids.remove(guid); } - class TaskEntry implements Map.Entry<Job, ArrayList<Job>> { + static class TaskEntry implements Map.Entry<Job, ArrayList<Job>> { public TaskEntry(Job key) { this.key = key; } diff --git a/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/hosts/AddClusterRM.java b/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/hosts/AddClusterRM.java index 76956dd..1c9ad70 100644 --- a/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/hosts/AddClusterRM.java +++ b/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/hosts/AddClusterRM.java @@ -154,7 +154,7 @@ private final Context context = new Context(); - public final class Context { + public static final class Context { public Enlistment enlistment; public VDSGroup clusterFoundByName; diff --git a/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/hosts/AddDataCenterRM.java b/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/hosts/AddDataCenterRM.java index cd8e59d..3776373 100644 --- a/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/hosts/AddDataCenterRM.java +++ b/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/hosts/AddDataCenterRM.java @@ -294,7 +294,7 @@ private final Context context = new Context(); - public final class Context { + public static final class Context { public Enlistment enlistment; public StoragePool dataCenterFoundByName; diff --git a/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/hosts/AddStorageDomainRM.java b/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/hosts/AddStorageDomainRM.java index 487e7e7..f2607a8 100644 --- a/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/hosts/AddStorageDomainRM.java +++ b/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/hosts/AddStorageDomainRM.java @@ -244,7 +244,7 @@ private final Context context = new Context(); - public final class Context { + public static final class Context { public Enlistment enlistment; public VDS host; diff --git a/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/hosts/ChangeHostClusterRM.java b/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/hosts/ChangeHostClusterRM.java index 362eaf3..f9c6dab 100644 --- a/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/hosts/ChangeHostClusterRM.java +++ b/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/hosts/ChangeHostClusterRM.java @@ -85,7 +85,7 @@ private final Context context = new Context(); - public final class Context { + public static final class Context { public Enlistment enlistment; public VdcReturnValueBase changeVDSClusterReturnValue; diff --git a/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/hosts/ConfigureLocalStorageModel.java b/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/hosts/ConfigureLocalStorageModel.java index 1972807..2f89c1a 100644 --- a/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/hosts/ConfigureLocalStorageModel.java +++ b/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/hosts/ConfigureLocalStorageModel.java @@ -628,7 +628,7 @@ private final Context context = new Context(); - public final class Context { + public static final class Context { public VDS host; public StoragePool hostDataCenter; diff --git a/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/users/UserEventNotifierListModel.java b/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/users/UserEventNotifierListModel.java index f02701f..374ae92 100644 --- a/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/users/UserEventNotifierListModel.java +++ b/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/users/UserEventNotifierListModel.java @@ -302,7 +302,7 @@ cancel(); } - private final class EventSubscriptionFrontendActionAsyncCallback implements IFrontendActionAsyncCallback { + private static final class EventSubscriptionFrontendActionAsyncCallback implements IFrontendActionAsyncCallback { private ArrayList<VdcActionParametersBase> toAddList; ArrayList<VdcActionParametersBase> toRemoveList; private int sucessCount = 0; diff --git a/frontend/webadmin/modules/userportal-gwtp/exclude-filters.xml b/frontend/webadmin/modules/userportal-gwtp/exclude-filters.xml index b36fe67..df93f0f 100644 --- a/frontend/webadmin/modules/userportal-gwtp/exclude-filters.xml +++ b/frontend/webadmin/modules/userportal-gwtp/exclude-filters.xml @@ -69,5 +69,12 @@ <Bug pattern="BC_UNCONFIRMED_CAST"/> </Match> + <!-- + Ignore non-static inner classes. + This pattern is used by GWT-generated code, which we have no control of + --> + <Match> + <Bug pattern="SIC_INNER_SHOULD_BE_STATIC" /> + </Match> - </FindBugsFilter> +</FindBugsFilter> diff --git a/frontend/webadmin/modules/userportal-gwtp/src/main/java/org/ovirt/engine/ui/userportal/widget/resources/VmTable.java b/frontend/webadmin/modules/userportal-gwtp/src/main/java/org/ovirt/engine/ui/userportal/widget/resources/VmTable.java index 5caf11d..a6a7a63 100644 --- a/frontend/webadmin/modules/userportal-gwtp/src/main/java/org/ovirt/engine/ui/userportal/widget/resources/VmTable.java +++ b/frontend/webadmin/modules/userportal-gwtp/src/main/java/org/ovirt/engine/ui/userportal/widget/resources/VmTable.java @@ -389,7 +389,7 @@ * This class guards, that only one row can be selected at a given time and the selection survives the refresh. The * single instance of this class should be used for more instances of EntityModelCellTable */ - class VmSingleSelectionModel extends SingleSelectionModel<EntityModel> { + static class VmSingleSelectionModel extends SingleSelectionModel<EntityModel> { private VM selectedVM = null; @@ -453,7 +453,7 @@ /** * An empty column - only for the header */ - private class EmptyColumn extends TextColumn<VM> { + private static class EmptyColumn extends TextColumn<VM> { @Override public String getValue(VM object) { diff --git a/frontend/webadmin/modules/webadmin/exclude-filters.xml b/frontend/webadmin/modules/webadmin/exclude-filters.xml index d6b38da..5adcc7f 100644 --- a/frontend/webadmin/modules/webadmin/exclude-filters.xml +++ b/frontend/webadmin/modules/webadmin/exclude-filters.xml @@ -261,6 +261,14 @@ <Bug pattern="BC_UNCONFIRMED_CAST"/> </Match> + <!-- + Ignore non-static inner classes. + This pattern is used by GWT-generated code, which we have no control of + --> + <Match> + <Bug pattern="SIC_INNER_SHOULD_BE_STATIC" /> + </Match> + <Match> <Class name="org.ovirt.engine.ui.webadmin.ApplicationConstants" /> <Bug pattern="SKIPPED_CLASS_TOO_BIG"/> -- To view, visit http://gerrit.ovirt.org/26227 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: Ifbceb1baad596211580c1289f49f7d12e33a8ecb Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Allon Mureinik <[email protected]> _______________________________________________ Engine-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-patches
