Remove abstract properties (#1781)

pkl-core/src/main/java/org/pkl/core/ast/VmModifier.java

@@ -71,6 +71,7 @@ public final class VmModifier {
 
   public static final int VALID_METHOD_MODIFIERS = ABSTRACT | LOCAL | EXTERNAL | CONST;
 
+  // for compat, properties may be parsed with abstract modifier but this is ignored
   public static final int VALID_PROPERTY_MODIFIERS =
       ABSTRACT | LOCAL | HIDDEN | EXTERNAL | FIXED | CONST;
 

pkl-core/src/main/java/org/pkl/core/ast/builder/AstBuilder.java

@@ -1476,7 +1476,7 @@ public class AstBuilder extends AbstractAstBuilder<Object> {
     var scope = (ModuleScope) symbolTable.getCurrentScope();
     scope.setModifiers(modifiers);
 
-    checkAbstractMembersAllowed(modifiers, mod.getProperties(), mod.getMethods());
+    checkAbstractMethodsAllowed(modifiers, mod.getMethods(), "module");
 
     // visit imports first so that we already have the object member name available
     var imports = mod.getImports();
@@ -1722,7 +1722,7 @@ public class AstBuilder extends AbstractAstBuilder<Object> {
           List<ClassProperty> properties = bodyNode != null ? bodyNode.getProperties() : List.of();
           List<ClassMethod> methods = bodyNode != null ? bodyNode.getMethods() : List.of();
           registerClassScopeNames(scope, properties, methods);
-          checkAbstractMembersAllowed(modifiers, properties, methods);
+          checkAbstractMethodsAllowed(modifiers, methods, "class");
 
           var supertypeCtx = clazz.getSuperClass();
 
@@ -1813,26 +1813,19 @@ public class AstBuilder extends AbstractAstBuilder<Object> {
     };
   }
 
-  private void checkAbstractMembersAllowed(
-      int enclosingModifiers, List<ClassProperty> properties, List<ClassMethod> methods) {
+  private void checkAbstractMethodsAllowed(
+      int enclosingModifiers, List<ClassMethod> methods, String context) {
     if (VmModifier.isAbstract(enclosingModifiers)) {
       return;
     }
-    for (var property : properties) {
-      checkMemberNotAbstract(property.getModifiers());
-    }
     for (var method : methods) {
-      checkMemberNotAbstract(method.getModifiers());
-    }
-  }
-
-  private void checkMemberNotAbstract(List<Modifier> modifiers) {
-    for (var modifier : modifiers) {
-      if (modifier.getValue() == ModifierValue.ABSTRACT) {
-        throw exceptionBuilder()
-            .evalError("abstractMemberInNonAbstractClass")
-            .withSourceSection(createSourceSection(modifier.span()))
-            .build();
+      for (var modifier : method.getModifiers()) {
+        if (modifier.getValue() == ModifierValue.ABSTRACT) {
+          throw exceptionBuilder()
+              .evalError("abstractMethodInNonAbstractType", context)
+              .withSourceSection(createSourceSection(modifier.span()))
+              .build();
+        }
       }
     }
   }
@@ -1878,9 +1871,11 @@ public class AstBuilder extends AbstractAstBuilder<Object> {
     var headerEnd = typeAnnotation != null ? typeAnnotation.span() : name.span();
     var headerSection = createSourceSection(headerStart.endWith(headerEnd));
 
-    var modifiers =
+    var fullModifiers =
         doVisitModifiers(
             modifierList, VmModifier.VALID_PROPERTY_MODIFIERS, "invalidPropertyModifier");
+    // for compat, properties may be abstract, but ignore this
+    var modifiers = fullModifiers & ~VmModifier.ABSTRACT;
 
     var isLocal = VmModifier.isLocal(modifiers);
     var propertyName = org.pkl.core.runtime.Identifier.property(name.getValue(), isLocal);
@@ -1899,12 +1894,6 @@ public class AstBuilder extends AbstractAstBuilder<Object> {
                   .withSourceSection(headerSection)
                   .build();
             }
-            if (VmModifier.isAbstract(modifiers)) {
-              throw exceptionBuilder()
-                  .evalError("abstractMemberCannotHaveBody")
-                  .withSourceSection(headerSection)
-                  .build();
-            }
             bodyNode = visitExpr(expr);
           } else if (!objectBodies.isEmpty()) { // prop { ... }
             if (typeAnnotation != null) {
@@ -1931,9 +1920,6 @@ public class AstBuilder extends AbstractAstBuilder<Object> {
               if (bodyNode instanceof LanguageAwareNode languageAwareNode) {
                 languageAwareNode.initLanguage(language);
               }
-            } else if (VmModifier.isAbstract(modifiers)) {
-              bodyNode =
-                  new CannotInvokeAbstractPropertyNode(headerSection, scope.getQualifiedName());
             } else {
               bodyNode = null; // will be given a default by UnresolvedPropertyNode
             }

pkl-core/src/main/java/org/pkl/core/ast/builder/CannotInvokeAbstractPropertyNode.java

@@ -1,36 +0,0 @@
-/*
- * Copyright © 2024 Apple Inc. and the Pkl project authors. All rights reserved.
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- *     https://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-package org.pkl.core.ast.builder;
-
-import com.oracle.truffle.api.CompilerDirectives;
-import com.oracle.truffle.api.frame.VirtualFrame;
-import com.oracle.truffle.api.source.SourceSection;
-import org.pkl.core.ast.ExpressionNode;
-
-public final class CannotInvokeAbstractPropertyNode extends ExpressionNode {
-  private final String propertyName;
-
-  public CannotInvokeAbstractPropertyNode(SourceSection section, String propertyName) {
-    super(section);
-    this.propertyName = propertyName;
-  }
-
-  @Override
-  public Object executeGeneric(VirtualFrame frame) {
-    CompilerDirectives.transferToInterpreter();
-    throw exceptionBuilder().evalError("cannotInvokeAbstractProperty", propertyName).build();
-  }
-}

pkl-core/src/main/java/org/pkl/core/runtime/MirrorFactories.java

@@ -180,8 +180,7 @@ public final class MirrorFactories {
         .addProperty(
             "defaultValue",
             property ->
-                property.isAbstract()
-                        || property.isExternal()
+                property.isExternal()
                         || property
                             .getInitializer()
                             .isUndefined(VmUtils.createEmptyMaterializedFrame())

pkl-core/src/main/resources/org/pkl/core/errorMessages.properties

@@ -43,9 +43,6 @@ Type alias definitions must not be cyclic.
 cannotInvokeAbstractMethod=\
 Cannot invoke abstract method `{0}`.
 
-cannotInvokeAbstractProperty=\
-Cannot invoke abstract property `{0}`.
-
 cannotInvokeSupermethodFromHere=\
 Cannot invoke a supermethod from here.
 
@@ -289,8 +286,8 @@ External members cannot have a body.
 abstractMemberCannotHaveBody=\
 Abstract members cannot have a body.
 
-abstractMemberInNonAbstractClass=\
-Cannot define an abstract member in a non-abstract class.\n\
+abstractMethodInNonAbstractType=\
+Cannot define an abstract method in a non-abstract {0}.\n\
 \n\
 A member can only be `abstract` if its enclosing class is also `abstract`.
 

pkl-core/src/test/files/LanguageSnippetTests/input/errors/abstractMemberInNonAbstractClass.pkl

@@ -1,5 +0,0 @@
-class Foo {
-  abstract bar: Int
-}
-
-res = new Foo { bar = 5 }

pkl-core/src/test/files/LanguageSnippetTests/input/errors/abstractMemberInNonAbstractModule.pkl

@@ -1 +0,0 @@
-abstract foo: Int

pkl-core/src/test/files/LanguageSnippetTests/input/errors/abstractMethodInNonAbstractModule.pkl

@@ -0,0 +1 @@
+abstract function foo(): Int

pkl-core/src/test/files/LanguageSnippetTests/input/internal/polymorphicCallSite.pkl

@@ -18,7 +18,7 @@ res3 = f(List(1, 2))
 res4 = f(Set(1, 2))
 
 abstract class Animal {
-  abstract size: String
+  size: String
   abstract function walk(): String
 }
 

pkl-core/src/test/files/LanguageSnippetTests/output/errors/abstractMemberInNonAbstractClass.err

@@ -1,8 +0,0 @@
-–– Pkl Error ––
-Cannot define an abstract member in a non-abstract class.
-
-x | abstract bar: Int
-    ^^^^^^^^
-at abstractMemberInNonAbstractClass#Foo (file:///$snippetsDir/input/errors/abstractMemberInNonAbstractClass.pkl)
-
-A member can only be `abstract` if its enclosing class is also `abstract`.

pkl-core/src/test/files/LanguageSnippetTests/output/errors/abstractMemberInNonAbstractModule.err

@@ -1,8 +0,0 @@
-–– Pkl Error ––
-Cannot define an abstract member in a non-abstract class.
-
-x | abstract foo: Int
-    ^^^^^^^^
-at abstractMemberInNonAbstractModule (file:///$snippetsDir/input/errors/abstractMemberInNonAbstractModule.pkl)
-
-A member can only be `abstract` if its enclosing class is also `abstract`.

pkl-core/src/test/files/LanguageSnippetTests/output/errors/abstractMethodInNonAbstractClass.err

@@ -1,5 +1,5 @@
 –– Pkl Error ––
-Cannot define an abstract member in a non-abstract class.
+Cannot define an abstract method in a non-abstract class.
 
 x | abstract function bar(): Int
     ^^^^^^^^

pkl-core/src/test/files/LanguageSnippetTests/output/errors/abstractMethodInNonAbstractModule.err

@@ -0,0 +1,8 @@
+–– Pkl Error ––
+Cannot define an abstract method in a non-abstract module.
+
+x | abstract function foo(): Int
+    ^^^^^^^^
+at abstractMethodInNonAbstractModule (file:///$snippetsDir/input/errors/abstractMethodInNonAbstractModule.pkl)
+
+A member can only be `abstract` if its enclosing class is also `abstract`.