mirror of
https://github.com/apple/pkl.git
synced 2026-06-11 00:02:47 +02:00
Reject abstract members in non-abstract classes (#1647)
Fixes #1614. ## Context A non-abstract `class` (or `module`) was allowed to declare `abstract` properties and methods. Because such an enclosing type is instantiable, an `abstract` member there can never be guaranteed an implementation — so the contradiction surfaced only as a runtime error when the member was accessed (`Cannot invoke abstract method`), or not at all. This makes it a compile-time error to declare an `abstract` member unless its enclosing class or module is also `abstract`. This is consistent with how Pkl already rejects instantiating an abstract class, and mirrors how Java and Kotlin treat abstract members. ## Before ```pkl class Foo { abstract bar: Int } res = new Foo { bar = 5 } // evaluated successfully (should fail) ``` ```pkl class Foo { abstract function bar(): Int } res = new Foo {} // evaluated successfully; res.bar() failed only at runtime ``` ## After ``` –– Pkl Error –– Cannot define an abstract member in a non-abstract class. 2 | abstract bar: Int ^^^^^^^^ at Foo A member can only be `abstract` if its enclosing class is also `abstract`. ``` ## Implementation - `AstBuilder` now validates, while building the AST, that a non-abstract class/module declares no `abstract` members. The check runs in both `visitClass` and `visitModule`, and the error points at the `abstract` keyword. - Adds the `abstractMemberInNonAbstractClass` error message. ## Scope: classes and modules The issue describes classes; I applied the same rule to modules as well, since a module is a class in Pkl and a non-abstract module is likewise directly evaluatable. Happy to narrow this to classes only if you'd prefer — it's a one-line change either way. The `moduleMethodModifiers` pkl-doc test fixture declared an abstract method at non-abstract module level (relying on the old behavior); it's updated to an `abstract module`, and its expected documentation output is regenerated. ## Tests - New `LanguageSnippetTests` error cases: abstract property in a class, abstract method in a class, and abstract member in a module. - `./gradlew build` passes (`pkl-core` and `pkl-doc` included). --------- Co-authored-by: Vinayak <vinayak@vama.app> Co-authored-by: Daniel Chao <daniel.h.chao@gmail.com>
This commit is contained in:
@@ -1441,6 +1441,8 @@ public class AstBuilder extends AbstractAstBuilder<Object> {
|
||||
var scope = (ModuleScope) symbolTable.getCurrentScope();
|
||||
scope.setModifiers(modifiers);
|
||||
|
||||
checkAbstractMembersAllowed(modifiers, mod.getProperties(), mod.getMethods());
|
||||
|
||||
// visit imports first so that we already have the object member name available
|
||||
var imports = mod.getImports();
|
||||
var importMembers = new ObjectMember[imports.size()];
|
||||
@@ -1685,6 +1687,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);
|
||||
|
||||
var supertypeCtx = clazz.getSuperClass();
|
||||
|
||||
@@ -1775,6 +1778,30 @@ public class AstBuilder extends AbstractAstBuilder<Object> {
|
||||
};
|
||||
}
|
||||
|
||||
private void checkAbstractMembersAllowed(
|
||||
int enclosingModifiers, List<ClassProperty> properties, List<ClassMethod> methods) {
|
||||
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();
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
private UnresolvedPropertyNode[] doVisitClassProperties(
|
||||
List<ClassProperty> propertyContexts, Set<String> propertyNames) {
|
||||
var propertyNodes = new UnresolvedPropertyNode[propertyContexts.size()];
|
||||
|
||||
@@ -286,6 +286,11 @@ 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\
|
||||
\n\
|
||||
A member can only be `abstract` if its enclosing class is also `abstract`.
|
||||
|
||||
methodNotDefined1=\
|
||||
Method `{0}` is not defined for argument type `{1}`.
|
||||
|
||||
|
||||
Vendored
+5
@@ -0,0 +1,5 @@
|
||||
class Foo {
|
||||
abstract bar: Int
|
||||
}
|
||||
|
||||
res = new Foo { bar = 5 }
|
||||
Vendored
+1
@@ -0,0 +1 @@
|
||||
abstract foo: Int
|
||||
Vendored
+5
@@ -0,0 +1,5 @@
|
||||
class Foo {
|
||||
abstract function bar(): Int
|
||||
}
|
||||
|
||||
res = new Foo {}
|
||||
Vendored
+8
@@ -0,0 +1,8 @@
|
||||
–– 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`.
|
||||
Vendored
+8
@@ -0,0 +1,8 @@
|
||||
–– 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`.
|
||||
Vendored
+8
@@ -0,0 +1,8 @@
|
||||
–– Pkl Error ––
|
||||
Cannot define an abstract member in a non-abstract class.
|
||||
|
||||
x | abstract function bar(): Int
|
||||
^^^^^^^^
|
||||
at abstractMethodInNonAbstractClass#Foo (file:///$snippetsDir/input/errors/abstractMethodInNonAbstractClass.pkl)
|
||||
|
||||
A member can only be `abstract` if its enclosing class is also `abstract`.
|
||||
+1
-1
@@ -1,5 +1,5 @@
|
||||
/// Module methods with different modifiers.
|
||||
module com.package1.moduleMethodModifiers
|
||||
abstract module com.package1.moduleMethodModifiers
|
||||
|
||||
/// Method with `abstract` modifier.
|
||||
abstract function method1(arg: String): Boolean
|
||||
|
||||
+1
-1
@@ -32,7 +32,7 @@
|
||||
</ul>
|
||||
<div id="_overview" class="anchor"> </div>
|
||||
<div id="_declaration" class="member">
|
||||
<div class="member-signature">module <span class="name-decl">com.package1.moduleMethodModifiers</span></div>
|
||||
<div class="member-signature">abstract module <span class="name-decl">com.package1.moduleMethodModifiers</span></div>
|
||||
<div class="doc-comment"><p>Module methods with different modifiers.</p></div>
|
||||
<dl class="member-info">
|
||||
<dt class="">Module URI:</dt>
|
||||
|
||||
+1
-1
@@ -32,7 +32,7 @@
|
||||
</ul>
|
||||
<div id="_overview" class="anchor"> </div>
|
||||
<div id="_declaration" class="member">
|
||||
<div class="member-signature">module <span class="name-decl">com.package1.moduleMethodModifiers</span></div>
|
||||
<div class="member-signature">abstract module <span class="name-decl">com.package1.moduleMethodModifiers</span></div>
|
||||
<div class="doc-comment"><p>Module methods with different modifiers.</p></div>
|
||||
<dl class="member-info">
|
||||
<dt class="">Module URI:</dt>
|
||||
|
||||
Reference in New Issue
Block a user