You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@groovy.apache.org by GitBox <gi...@apache.org> on 2021/07/27 13:09:19 UTC
[GitHub] [groovy] paulk-asert opened a new pull request #1606: GROOVY-10148: Groovy should not allow classes to extend sealed Java c…
paulk-asert opened a new pull request #1606:
URL: https://github.com/apache/groovy/pull/1606
…lasses
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@groovy.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [groovy] sonatype-lift[bot] commented on a change in pull request #1606: GROOVY-10148: Groovy should not allow classes to extend sealed Java c…
Posted by GitBox <gi...@apache.org>.
sonatype-lift[bot] commented on a change in pull request #1606:
URL: https://github.com/apache/groovy/pull/1606#discussion_r680271839
##########
File path: src/main/java/org/codehaus/groovy/classgen/ClassCompletionVerifier.java
##########
@@ -309,23 +313,79 @@ private void checkAbstractDeclaration(MethodNode methodNode) {
methodNode.getTypeDescriptor() + "' must not be abstract.", methodNode);
}
- private void checkClassForOverwritingFinal(ClassNode cn) {
+ private void checkClassForExtendingFinalOrSealed(ClassNode cn) {
+ boolean sealed = Boolean.TRUE.equals(cn.getNodeMetaData(Sealed.class));
+ if (sealed && cn.getPermittedSubclasses().isEmpty()) {
+ addError("Sealed " + getDescription(cn) + " has no explicit or implicit permitted classes.", cn);
+ return;
+ }
+ boolean isFinal = isFinal(cn.getModifiers());
+ if (sealed && isFinal) {
+ addError("The " + getDescription(cn) + " cannot be both final and sealed.", cn);
+ return;
+ }
+ boolean nonSealed = Boolean.TRUE.equals(cn.getNodeMetaData(NonSealed.class));
ClassNode superCN = cn.getSuperClass();
- if (superCN == null) return;
- if (!isFinal(superCN.getModifiers())) return;
- String msg = "You are not allowed to overwrite the final " + getDescription(superCN) + ".";
- addError(msg, cn);
+ boolean sealedSuper = superCN != null && superCN.isSealed();
+ boolean sealedInterface = Arrays.stream(cn.getInterfaces()).anyMatch(ClassNode::isSealed);
+ if (nonSealed && !(sealedSuper || sealedInterface)) {
+ addError("The " + getDescription(cn) + " cannot be non-sealed as it has no sealed parent.", cn);
+ return;
+ }
+ if (sealedSuper || sealedInterface) {
+ if (sealed && nonSealed) {
+ addError("The " + getDescription(cn) + " cannot be both sealed and non-sealed.", cn);
+ return;
+ }
+ if (isFinal && nonSealed) {
+ addError("The " + getDescription(cn) + " cannot be both final and non-sealed.", cn);
+ return;
+ }
+ if (sealedSuper) {
+ checkSealedParent(cn, superCN, isFinal, nonSealed);
+ }
+ if (sealedInterface) {
+ for (ClassNode candidate : cn.getInterfaces()) {
+ if (candidate.isSealed()) {
+ checkSealedParent(cn, candidate, isFinal, nonSealed);
+ }
+ }
+ }
+ }
+ if (superCN == null || !isFinal(superCN.getModifiers())) return;
+ addError("You are not allowed to extend the final " + getDescription(superCN) + ".", cn);
+ }
+
+ private void checkSealedParent(ClassNode cn, ClassNode parent, boolean isFinal, boolean nonSealed) {
+ boolean found = false;
+ for (ClassNode permitted : parent.getPermittedSubclasses()) {
+ if (permitted.equals(cn)) {
+ found = true;
+ break;
+ }
+ }
+ if (!found) {
+ addError("The " + getDescription(cn) + " is not a permitted subclass of the sealed " + getDescription(parent) + ".", cn);
+ return;
+ }
+ boolean explicitlyMarked = nonSealed || cn.isSealed() || isFinal;
+ if (!explicitlyMarked) {
+ addError("The " + getDescription(cn) + " being a child of sealed " + getDescription(parent) + " must be marked final, sealed, or non-sealed.", cn);
+ }
}
private void checkImplementsAndExtends(ClassNode node) {
- ClassNode cn = node.getSuperClass();
- if (cn.isInterface() && !node.isInterface()) {
- addError("You are not allowed to extend the " + getDescription(cn) + ", use implements instead.", node);
+ ClassNode sn = node.getSuperClass();
+ if (sn.isInterface() && !node.isInterface()) {
Review comment:
*NULL_DEREFERENCE:* object `sn` last assigned on line 378 could be null and is dereferenced at line 379.
(at-me [in a reply](https://help.sonatype.com/lift) with `help` or `ignore`)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@groovy.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [groovy] danielsun1106 commented on pull request #1606: GROOVY-10148: Groovy should not allow classes to extend sealed Java c…
Posted by GitBox <gi...@apache.org>.
danielsun1106 commented on pull request #1606:
URL: https://github.com/apache/groovy/pull/1606#issuecomment-898911180
The PR has been merged: https://github.com/apache/groovy/commit/5fd04e30558d9154f53e005d6329698b3d65488b
so close it now.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@groovy.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [groovy] danielsun1106 closed pull request #1606: GROOVY-10148: Groovy should not allow classes to extend sealed Java c…
Posted by GitBox <gi...@apache.org>.
danielsun1106 closed pull request #1606:
URL: https://github.com/apache/groovy/pull/1606
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@groovy.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org