Allon Mureinik has uploaded a new change for review. Change subject: core: Remove final members from common package ......................................................................
core: Remove final members from common package Since GWT RPC does not handle final members properly, there is a need to implement a mechanism to enforce that no member in this package will be made final by mistake (unless it's a static final constant), as was done, e.g., in patch I53d6e5bbd66c4d67ec37326b545b65f086600169. This patch offers such a mechanism by providing a new checkstyle check which is only configured to run on the common and compat modules. Change-Id: I809e7f670cf86b47dbd7ae1fc6f73eff9837a129 Signed-off-by: Allon Mureinik <[email protected]> --- M backend/manager/modules/common/pom.xml M backend/manager/modules/compat/pom.xml M build-tools-root/checkstyles/src/main/resources/checkstyle.xml A build-tools-root/ovirt-checkstyle-extension/src/main/java/checks/NoFinalMemberCheck.java 4 files changed, 70 insertions(+), 0 deletions(-) git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/52/16252/1 diff --git a/backend/manager/modules/common/pom.xml b/backend/manager/modules/common/pom.xml index 888c52e..071cf5b 100644 --- a/backend/manager/modules/common/pom.xml +++ b/backend/manager/modules/common/pom.xml @@ -64,6 +64,12 @@ </executions> </plugin> <plugin> + <artifactId>maven-checkstyle-plugin</artifactId> + <configuration> + <propertyExpansion>disallowFinals=true</propertyExpansion> + </configuration> + </plugin> + <plugin> <!-- The enforcer plugin is used to ban us from using artifacts that will fail diff --git a/backend/manager/modules/compat/pom.xml b/backend/manager/modules/compat/pom.xml index 180e48d..19f455b 100644 --- a/backend/manager/modules/compat/pom.xml +++ b/backend/manager/modules/compat/pom.xml @@ -32,6 +32,12 @@ </execution> </executions> </plugin> + <plugin> + <artifactId>maven-checkstyle-plugin</artifactId> + <configuration> + <propertyExpansion>disallowFinals=true</propertyExpansion> + </configuration> + </plugin> <!-- Create the JBoss module: --> <plugin> diff --git a/build-tools-root/checkstyles/src/main/resources/checkstyle.xml b/build-tools-root/checkstyles/src/main/resources/checkstyle.xml index 1f0992d..a4026fc 100644 --- a/build-tools-root/checkstyles/src/main/resources/checkstyle.xml +++ b/build-tools-root/checkstyles/src/main/resources/checkstyle.xml @@ -22,5 +22,8 @@ <module name="checks.NlsCheck"> <property name="run" value="${runNlsCheck}" default="false"/> </module> + <module name="checks.NoFinalMemberCheck"> + <property name="run" value="${disallowFinals}" default="false"/> + </module> </module> </module> diff --git a/build-tools-root/ovirt-checkstyle-extension/src/main/java/checks/NoFinalMemberCheck.java b/build-tools-root/ovirt-checkstyle-extension/src/main/java/checks/NoFinalMemberCheck.java new file mode 100644 index 0000000..017a7e3 --- /dev/null +++ b/build-tools-root/ovirt-checkstyle-extension/src/main/java/checks/NoFinalMemberCheck.java @@ -0,0 +1,55 @@ +package checks; + +import com.puppycrawl.tools.checkstyle.api.Check; +import com.puppycrawl.tools.checkstyle.api.DetailAST; +import com.puppycrawl.tools.checkstyle.api.TokenTypes; + +/** + * Since GWT can't handle member variables which have the private modifier, this check was added to make sure it is not + * used in the packages that need to undergo GWT compilation. + */ +public class NoFinalMemberCheck extends Check { + + private boolean run = true; + + /** This check is not configurable */ + @Override + public int[] getDefaultTokens() { + return new int[] { TokenTypes.VARIABLE_DEF}; + } + + @Override + public void visitToken(DetailAST aAST) { + if (run) { + if (aAST.getType() == TokenTypes.VARIABLE_DEF // A variable + && aAST.getParent().getType() == TokenTypes.OBJBLOCK // which is a class variable + && aAST.getParent().getParent().getType() != TokenTypes.ENUM_DEF // and not in an enum + ) { +// System.out.println("Analyzing " + aAST.getText() + " " +aAST.toString() +" parent is " + aAST.getParent().getText()); +// System.err.println("Analyzing " + aAST.getText() + " " +aAST.toString() +" parent is " + aAST.getParent().getText()); +// DetailAST previousSibling = aAST.getPreviousSibling(); +// if (previousSibling != null) { +// int previousSiblingType = previousSibling.getType(); +// if (previousSiblingType == TokenTypes.LPAREN || previousSiblingType == TokenTypes.COMMA) { +// return; +// } +// } + + // find the modifiers + DetailAST child = aAST.getFirstChild(); + while (child != null) { + // final is only allowed for statics (i.e., constants) + if (child.branchContains(TokenTypes.FINAL) && + !child.branchContains(TokenTypes.LITERAL_STATIC)) { + log(child.getLineNo(), child.getColumnNo(), "final variables are not allowed"); + } + child = child.getNextSibling(); + } + } + } + } + + public void setRun(boolean run) { + this.run = run; + } +} -- To view, visit http://gerrit.ovirt.org/16252 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I809e7f670cf86b47dbd7ae1fc6f73eff9837a129 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
