Relax Visibility of Generated Members

cdlang/src/cdGenIntTest/java/getter/GetterDecoratorResultTest.java

@@ -20,11 +20,11 @@ public void test() throws Exception {
     obj.__setMyInt(42);
     Assertions.assertEquals(42, obj.getMyInt());
 
-    // Check if the boolean is prefixed with is & has the "public" default visibility (via default)
+    // Check if the boolean is prefixed with is and has relaxed protected visibility
     Method isMyBool = TestGetter.TestGetterC.class.getDeclaredMethod("isMyBool");
-    Assertions.assertTrue(Modifier.isPublic(isMyBool.getModifiers()));
+    Assertions.assertFalse(Modifier.isPublic(isMyBool.getModifiers()));
     Assertions.assertFalse(Modifier.isPrivate(isMyBool.getModifiers()));
-    Assertions.assertFalse(Modifier.isProtected(isMyBool.getModifiers()));
+    Assertions.assertTrue(Modifier.isProtected(isMyBool.getModifiers()));
 
     // Test NoGetter / public
     Assertions.assertEquals(0, obj.pubX);

cdlang/src/main/java/de/monticore/cd/codegen/decorators/GetterDecorator.java

@@ -80,7 +80,7 @@
   }
 
   protected void logAlreadyExistingWarning(ASTCDAttribute attribute) {
     Log.warn("0xTODO: Unable to decorate setter of `" + attribute.getName()
         + "` as such a method already exists.", attribute.get_SourcePositionStart(), attribute
             .get_SourcePositionEnd());
 
@@ -91,8 +91,8 @@
     String name = (MCTypeFacade.getInstance().isBooleanType(attribute.getMCType()) ? "is" : "get")
         + StringTransformations.capitalize(attribute.getName());
     ASTMCType type = attribute.getMCType().deepClone();
-    ASTCDMethod method = CDMethodFacade.getInstance().createMethod(attribute.getModifier()
-        .deepClone(), type, name);
+    ASTCDMethod method = CDMethodFacade.getInstance().createMethod(getSourceModifier(attribute),
+        type, name);
     glexOpt.ifPresent(glex -> glex.replaceTemplate(EMPTY_BODY, method, new TemplateHookPoint(
         "methods.Get", attribute)));
     method.getModifier().setAbstract(attribute.getModifier().isDerived());
@@ -115,8 +115,8 @@
 
     String generatedErrorCode = getCDGenService().getGeneratedErrorCode(attribute.getName()
         + attribute.getMCType().printType());
-    ASTCDMethod getMethod = CDMethodFacade.getInstance().createMethod(attribute.getModifier()
-        .deepClone(), type, name);
+    ASTCDMethod getMethod = CDMethodFacade.getInstance().createMethod(getSourceModifier(attribute),
+        type, name);
     String nativeAttributeName = StringUtils.capitalize(getCDGenService().getNativeAttributeName(
         attribute.getName()));
     glexOpt.ifPresent(glex -> glex.replaceTemplate(EMPTY_BODY, getMethod, new TemplateHookPoint(
@@ -134,8 +134,8 @@
 
   protected Optional<MethodInformation> decorateOptionalIsPresent(ASTCDType decoratedType,
       ASTCDAttribute attribute) {
-    ASTCDMethod isPresentMethod = CDMethodFacade.getInstance().createMethod(attribute.getModifier()
-        .deepClone(), MCTypeFacade.getInstance().createBooleanType(), "isPresent"
+    ASTCDMethod isPresentMethod = CDMethodFacade.getInstance().createMethod(getSourceModifier(
+        attribute), MCTypeFacade.getInstance().createBooleanType(), "isPresent"
             + StringTransformations.capitalize(attribute.getName()));
     glexOpt.ifPresent(glex -> glex.replaceTemplate(EMPTY_BODY, isPresentMethod,
         new TemplateHookPoint("methods.opt.IsPresent4Opt", attribute)));
@@ -154,8 +154,8 @@
     String name = "get" + StringTransformations.capitalize(attribute.getName());
     ASTMCType type = getCDGenService().getFirstTypeArgument(attribute.getMCType()).deepClone();
 
-    ASTCDMethod getListMethod = CDMethodFacade.getInstance().createMethod(attribute.getModifier()
-        .deepClone(), MCTypeFacadeFix.createSetTypeOf(type), name);
+    ASTCDMethod getListMethod = CDMethodFacade.getInstance().createMethod(getSourceModifier(
+        attribute), MCTypeFacadeFix.createSetTypeOf(type), name);
     glexOpt.ifPresent(glex -> glex.replaceTemplate(EMPTY_BODY, getListMethod, new TemplateHookPoint(
         "methods.Get", attribute)));
     getListMethod.getModifier().setAbstract(attribute.getModifier().isDerived());
@@ -175,8 +175,8 @@
     String name = "get" + StringTransformations.capitalize(attribute.getName());
     ASTMCType type = getCDGenService().getFirstTypeArgument(attribute.getMCType()).deepClone();
 
-    ASTCDMethod getListMethod = CDMethodFacade.getInstance().createMethod(attribute.getModifier()
-        .deepClone(), MCTypeFacadeFix.createListTypeOf(type), name);
+    ASTCDMethod getListMethod = CDMethodFacade.getInstance().createMethod(getSourceModifier(
+        attribute), MCTypeFacadeFix.createListTypeOf(type), name);
     glexOpt.ifPresent(glex -> glex.replaceTemplate(EMPTY_BODY, getListMethod, new TemplateHookPoint(
         "methods.Get", attribute)));
     getListMethod.getModifier().setAbstract(attribute.getModifier().isDerived());
@@ -224,6 +224,7 @@
                   capitalizedAttributeNameWithS), String.format(HASHCODE,
                       capitalizedAttributeNameWithS))) {
         ASTCDMethod method = CDMethodFacade.getInstance().createMethodByDefinition(signature);
+        method.setModifier(getSourceModifier(attribute));
         addToClass(decoratedType, method);
         this.glexOpt.ifPresent(glex -> glex.replaceTemplate(EMPTY_BODY, method,
             createListImplementation(method, capitalizedAttributeNameWithOutS)));
@@ -237,7 +238,7 @@
                             capitalizedAttributeNameWithS), String.format(SUBLIST, attributeType,
                                 capitalizedAttributeNameWithS))) {
           ASTCDMethod method = CDMethodFacade.getInstance().createMethodByDefinition(signature);
-          method.setModifier(attribute.getModifier().deepClone());
+          method.setModifier(getSourceModifier(attribute));
           addToClass(decoratedType, method);
           this.glexOpt.ifPresent(glex -> glex.replaceTemplate(EMPTY_BODY, method,
               createListImplementation(method, capitalizedAttributeNameWithOutS)));

cdlang/src/main/java/de/monticore/cd/codegen/decorators/NavigableSetterDecorator.java

@@ -125,7 +125,7 @@
         case SET:
           // Add set${role}Local method for *
           if (otherSideAttrInfo.isOrdered()) {
             Log.warn("0xTODO: Ordered navigable setters are still WIP", attribute
                 .get_SourcePositionStart());
           }
           else {
@@ -152,7 +152,7 @@
           callLocal(thisSideAttrInfo, methods, role, "add", thisName);
           break;
         default:
           Log.error("0xTODO: Unhandled multiplicity " + otherSideAttrInfo.getMultiplicity()
               + " in NavigableSetter", attribute.get_SourcePositionStart());
       }
 
@@ -199,7 +199,7 @@
                   .getName(), method))));
     }
     else {
       Log.warn("0xTODO: Unhandled multiplicity" + thisSideAttrInfo.getMultiplicity()
           + " in NavigableSetter#callLocal", role.getSourcePosition());
     }
   }
@@ -208,8 +208,12 @@
       SetterDecorator.SetterMethodKind kind, String templateName, String methodName,
       List<ASTCDParameter> params, String paramName, Object... templateParams) {
 
-    ASTCDMethod method = CDMethodFacade.getInstance().createMethod(role.getAssocSide().getModifier()
-        .deepClone(), methodName, params);
+    var modifier = role.getAssocSide().getModifier().deepClone();
+    // Local synchronization methods can be called by an association end in another package.
+    modifier.setPrivate(false);
+    modifier.setProtected(false);
+    modifier.setPublic(true);
+    ASTCDMethod method = CDMethodFacade.getInstance().createMethod(modifier, methodName, params);
     glexOpt.ifPresent(glex -> glex.replaceTemplate(EMPTY_BODY, method, new TemplateHookPoint(
         templateName, templateParams)));
 

cdlang/src/main/java/de/monticore/cd/codegen/decorators/SetterDecorator.java

@@ -166,13 +166,13 @@
       SetterMethodKind kind, String templateName, String methodName, List<ASTCDParameter> params,
       Object... templateParams) {
 
-    ASTCDMethod method = CDMethodFacade.getInstance().createMethod(attribute.getModifier()
-        .deepClone(), methodName, params);
+    ASTCDMethod method = CDMethodFacade.getInstance().createMethod(getSourceModifier(attribute),
+        methodName, params);
     glexOpt.ifPresent(glex -> glex.replaceTemplate(EMPTY_BODY, method, new TemplateHookPoint(
         templateName, templateParams)));
 
     if (!addToClass(decParent, method)) {
       Log.warn("0xTODO: Unable to decorate setter of `" + attribute.getName()
           + "` as such a method already exists.", attribute.get_SourcePositionStart(), attribute
               .get_SourcePositionEnd());
       return Optional.empty();

cdlang/src/main/java/de/monticore/cd/codegen/decorators/data/AbstractDecorator.java

@@ -7,6 +7,7 @@
 import de.monticore.cd4codebasis._ast.ASTCDMethod;
 import de.monticore.cdbasis._ast.*;
 import de.monticore.generating.templateengine.GlobalExtensionManagement;
+import de.monticore.umlmodifier._ast.ASTModifier;
 import java.util.Optional;
 
 /**
@@ -71,6 +72,20 @@ else if (member instanceof ASTCDAttribute attribute) {
     return true;
   }
 
+  /**
+   * Returns the original model modifier for an attribute. For attributes generated from association
+   * roles, the association side modifier is the source of truth because accessor decorators must
+   * derive their visibility from the modeled role, not from the already generated field.
+   *
+   * @param attribute the generated or modeled attribute
+   * @return the source modifier to use for visibility-sensitive derived members
+   */
+  protected ASTModifier getSourceModifier(ASTCDAttribute attribute) {
+    var role = decoratorData.fieldToRoles.get(attribute.getSymbol());
+    return role == null ? attribute.getModifier().deepClone() : role.getAssocSide().getModifier()
+        .deepClone();
+  }
+
   public CDGenService getCDGenService() { return decoratorData.cdGenService; }
 
   /** For Decorators not specifying any additional data */

cdlang/src/main/java/de/monticore/cd/codegen/trafo/DefaultVisibilityPublicTrafo.java

@@ -1,10 +1,64 @@
 /* (c) https://github.com/MontiCore/monticore */
 package de.monticore.cd.codegen.trafo;
 
+import de.monticore.cdbasis._ast.ASTCDAttribute;
+import de.monticore.cdbasis._ast.ASTCDClass;
+import de.monticore.cdbasis._visitor.CDBasisVisitor2;
+import de.monticore.cd4codebasis._ast.ASTCDConstructor;
+import de.monticore.cd4codebasis._ast.ASTCDMethod;
+import de.monticore.cd4codebasis._visitor.CD4CodeBasisVisitor2;
+import de.monticore.cdassociation._ast.ASTCDAssocSide;
+import de.monticore.cdassociation._visitor.CDAssociationVisitor2;
 import de.monticore.umlmodifier._ast.ASTModifier;
 import de.monticore.umlmodifier._visitor.UMLModifierVisitor2;
 
-public class DefaultVisibilityPublicTrafo implements UMLModifierVisitor2 {
+public class DefaultVisibilityPublicTrafo implements UMLModifierVisitor2, CDBasisVisitor2,
+    CD4CodeBasisVisitor2, CDAssociationVisitor2 {
+
+  protected int classDepth;
+
+  @Override
+  public void visit(ASTCDClass node) {
+    classDepth++;
+  }
+
+  @Override
+  public void endVisit(ASTCDClass node) {
+    classDepth--;
+  }
+
+  @Override
+  public void visit(ASTCDAttribute node) {
+    relaxClassMemberVisibility(node.getModifier());
+  }
+
+  @Override
+  public void visit(ASTCDMethod node) {
+    relaxClassMemberVisibility(node.getModifier());
+  }
+
+  @Override
+  public void visit(ASTCDConstructor node) {
+    relaxClassMemberVisibility(node.getModifier());
+  }
+
+  @Override
+  public void visit(ASTCDAssocSide node) {
+    relaxVisibility(node.getModifier());
+  }
+
+  protected void relaxClassMemberVisibility(ASTModifier modifier) {
+    if (classDepth > 0) {
+      relaxVisibility(modifier);
+    }
+  }
+
+  protected void relaxVisibility(ASTModifier modifier) {
+    if (!modifier.isPublic() && !modifier.isProtected()) {
+      modifier.setPrivate(false);
+      modifier.setProtected(true);
+    }
+  }
 
   @Override
   public void visit(ASTModifier node) {

cdlang/src/main/java/de/monticore/cdgen/CDGenTool.java

@@ -453,7 +453,11 @@ public Collection<ASTCDCompilationUnit> trafoBeforeSymtab(Collection<ASTCDCompil
     asts.forEach(ast -> ast.accept(trafo.getTraverser()));
     // TODO: Have this be done via the config-options (#4310)
     var t = CD4CodeMill.inheritanceTraverser();
-    t.add4UMLModifier(new DefaultVisibilityPublicTrafo());
+    var visibilityTrafo = new DefaultVisibilityPublicTrafo();
+    t.add4CDBasis(visibilityTrafo);
+    t.add4CD4CodeBasis(visibilityTrafo);
+    t.add4CDAssociation(visibilityTrafo);
+    t.add4UMLModifier(visibilityTrafo);
     asts.forEach(ast -> ast.accept(t));
     return asts;
   }

cdlang/src/test/java/de/monticore/cd/cdgen/BuilderDecoratorTest.java

@@ -63,7 +63,7 @@ enum TestEnum {
           }
           interface Level1Interface;
           class Level2class implements Level1Interface{
-            int myInt;
+            public int myInt;
           }
         }""");
 

cdlang/src/test/java/de/monticore/cd/cdgen/DefaultCD2PojoDecoratorTest.java

@@ -3,13 +3,23 @@
 
 import de.monticore.cd.codegen.DecoratorConfig;
 import de.monticore.cd4code.CD4CodeMill;
+import de.monticore.cdbasis._ast.ASTCDClass;
 import de.monticore.generating.GeneratorSetup;
 import de.monticore.generating.templateengine.GlobalExtensionManagement;
 import de.monticore.generating.templateengine.TemplateController;
 import de.monticore.generating.templateengine.TemplateHookPoint;
 import de.monticore.runtime.junit.MCAssertions;
-
+import de.monticore.umlmodifier._ast.ASTModifier;
+import de.se_rwth.commons.logging.Log;
+import java.io.File;
+import java.nio.file.Files;
+import java.nio.file.Path;
 import java.util.ArrayList;
+import java.util.List;
+import java.util.stream.Collectors;
+import javax.tools.DiagnosticCollector;
+import javax.tools.JavaFileObject;
+import javax.tools.ToolProvider;
 import org.junit.jupiter.api.Assertions;
 import org.junit.jupiter.api.Test;
 
@@ -19,6 +29,103 @@
  */
 public class DefaultCD2PojoDecoratorTest extends AbstractDecoratorTest {
 
+  protected void compileGeneratedSources() throws Exception {
+    var compiler = ToolProvider.getSystemJavaCompiler();
+    Assertions.assertNotNull(compiler, "The generation regression test requires a JDK");
+    var diagnostics = new DiagnosticCollector<JavaFileObject>();
+    Path classes = outputDir.toPath().resolve("compiled");
+    Files.createDirectories(classes);
+    try (
+        var files = Files.walk(outputDir.toPath());
+        var manager = compiler.getStandardFileManager(diagnostics, null, null)
+    ) {
+      List<File> sources = files.filter(path -> path.toString().endsWith(".java")).map(Path::toFile)
+          .collect(Collectors.toList());
+      String loggingPath = new File(Log.class.getProtectionDomain().getCodeSource().getLocation()
+          .toURI()).getAbsolutePath();
+      Assertions.assertTrue(compiler.getTask(null, manager, diagnostics, List.of("-d", classes
+          .toString(), "-classpath", loggingPath), null, manager.getJavaFileObjectsFromFiles(
+              sources)).call(), diagnostics.getDiagnostics().toString());
+    }
+  }
+
+  @Test
+  public void testRelaxedClassMemberVisibility() throws Exception {
+    var ast = CD4CodeMill.parser().parse_String("""
+        classdiagram RelaxedVisibility {
+          class A {
+            private A();
+            private String privateValue;
+            String packageValue;
+            private void privateMethod();
+            void packageMethod();
+            public void publicMethod();
+            protected void protectedMethod();
+          }
+          class B {}
+          association A -> (privateBs) B [*] private;
+          association A -> (packageBs) B [*];
+          association A -> (privateOrderedBs) B [*] private {ordered};
+          association A -> (packageOrderedBs) B [*] {ordered};
+          association A -> (privateOptionalB) B [0..1] private;
+          association A -> (packageOptionalB) B [0..1];
+          association A -> (publicBs) B [*] public;
+          package Inner {
+            class InnerB {}
+          }
+          association A (innerB) <-> (a) Inner.InnerB;
+        }
+        """).orElseThrow();
+
+    var result = doTest(ast);
+    var clazz = result.getDecoratedCD().getCDDefinition().getCDClassesList().get(0);
+    assertProtectedOnly(clazz.getCDConstructorList().get(0).getModifier());
+    for (String attributeName : List.of("privateValue", "packageValue", "privateBs", "packageBs",
+        "privateOrderedBs", "packageOrderedBs", "privateOptionalB", "packageOptionalB")) {
+      var modifier = clazz.getCDAttributeList().stream().filter(attribute -> attribute.getName()
+          .equals(attributeName)).findFirst().orElseThrow().getModifier();
+      assertProtectedOnly(modifier);
+    }
+    for (String methodName : List.of("privateMethod", "packageMethod", "getPrivateValue",
+        "setPrivateValue", "getPackageValue", "setPackageValue")) {
+      var modifier = clazz.getCDMethodList().stream().filter(method -> method.getName().equals(
+          methodName)).findFirst().orElseThrow().getModifier();
+      assertProtectedOnly(modifier);
+    }
+    for (String memberSuffix : List.of("PrivateBs", "PackageBs", "PrivateOrderedBs",
+        "PackageOrderedBs", "PrivateOptionalB", "PackageOptionalB")) {
+      var generatedMethods = clazz.getCDMethodList().stream().filter(method -> method.getName()
+          .endsWith(memberSuffix)).collect(Collectors.toList());
+      Assertions.assertFalse(generatedMethods.isEmpty(), memberSuffix);
+      generatedMethods.forEach(method -> assertProtectedOnly(method.getModifier()));
+    }
+    Assertions.assertTrue(clazz.getCDMethodList().stream().filter(method -> method.getName().equals(
+        "publicMethod")).findFirst().orElseThrow().getModifier().isPublic());
+    Assertions.assertTrue(clazz.getCDMethodList().stream().filter(method -> method.getName().equals(
+        "protectedMethod")).findFirst().orElseThrow().getModifier().isProtected());
+    var publicAssociationMethods = clazz.getCDMethodList().stream().filter(method -> method
+        .getName().endsWith("PublicBs")).collect(Collectors.toList());
+    Assertions.assertFalse(publicAssociationMethods.isEmpty());
+    publicAssociationMethods.forEach(method -> Assertions.assertTrue(method.getModifier()
+        .isPublic()));
+    var innerClass = (ASTCDClass) result.getDecoratedCD().getCDDefinition().getCDPackagesList().get(
+        0).getCDElementList().get(0);
+    for (var associationClass : List.of(clazz, innerClass)) {
+      var localMethods = associationClass.getCDMethodList().stream().filter(method -> method
+          .getName().endsWith("Local")).collect(Collectors.toList());
+      Assertions.assertFalse(localMethods.isEmpty());
+      localMethods.forEach(method -> Assertions.assertTrue(method.getModifier().isPublic()));
+    }
+    MCAssertions.assertNoFindings();
+    compileGeneratedSources();
+  }
+
+  protected void assertProtectedOnly(ASTModifier modifier) {
+    Assertions.assertTrue(modifier.isProtected());
+    Assertions.assertFalse(modifier.isPrivate());
+    Assertions.assertFalse(modifier.isPublic());
+  }
+
   @Test
   public void testAll() throws Exception {
     var opt = CD4CodeMill.parser().parse_String("classdiagram TestDefaultCD2Pojo {\n"

cdlang/src/test/java/de/monticore/cd/cdgen/GetterDecoratorTest.java

@@ -25,8 +25,8 @@ public void testGetter() throws Exception {
            boolean myBool; public int myInt;
            <<noGetter>> public int pubX;
          }
-         public association TestGetterC -> (roleB) Other [*];
-         public association TestGetterC -> (orderedRole) Other [*] {ordered};
+         public association TestGetterC -> (roleB) Other [*] public;
+         public association TestGetterC -> (orderedRole) Other [*] public {ordered};
          <<getter>> public class Other {
          }
          <<getter>> public class AlreadyExisting {