diff --git a/de.peeeq.wurstscript/parserspec/wurstscript.parseq b/de.peeeq.wurstscript/parserspec/wurstscript.parseq index c2745cbab..3ef44f5b7 100644 --- a/de.peeeq.wurstscript/parserspec/wurstscript.parseq +++ b/de.peeeq.wurstscript/parserspec/wurstscript.parseq @@ -297,7 +297,6 @@ NameDef = NamedScope | VarDef | TypeDef | ModuleDef | ModuleInstanciation | Func VisibilityModifier = VisibilityPublic(de.peeeq.wurstscript.parser.WPos source) | VisibilityPrivate(de.peeeq.wurstscript.parser.WPos source) - | VisibilityPublicread(de.peeeq.wurstscript.parser.WPos source) | VisibilityProtected(de.peeeq.wurstscript.parser.WPos source) | VisibilityDefault(de.peeeq.wurstscript.parser.WPos source) @@ -310,6 +309,7 @@ Modifier = | ModOverride(de.peeeq.wurstscript.parser.WPos source) | ModAbstract(de.peeeq.wurstscript.parser.WPos source) | ModConstant(de.peeeq.wurstscript.parser.WPos source) + | ModReadonly(de.peeeq.wurstscript.parser.WPos source) | WurstDoc(@ignoreForEquality de.peeeq.wurstscript.parser.WPos source, String rawComment) | ModVararg(de.peeeq.wurstscript.parser.WPos source) @@ -582,9 +582,9 @@ HasModifier.attrIsPublic() returns boolean implemented by de.peeeq.wurstscript.attributes.ModifiersHelper.isPublic -HasModifier.attrIsPublicRead() +HasModifier.attrIsReadonly() returns boolean - implemented by de.peeeq.wurstscript.attributes.ModifiersHelper.isPublicRead + implemented by de.peeeq.wurstscript.attributes.ModifiersHelper.isReadonly HasModifier.attrIsPrivate() returns boolean diff --git a/de.peeeq.wurstscript/src/main/antlr/de/peeeq/wurstscript/antlr/Wurst.g4 b/de.peeeq.wurstscript/src/main/antlr/de/peeeq/wurstscript/antlr/Wurst.g4 index 22684d4e6..a3583c040 100644 --- a/de.peeeq.wurstscript/src/main/antlr/de/peeeq/wurstscript/antlr/Wurst.g4 +++ b/de.peeeq.wurstscript/src/main/antlr/de/peeeq/wurstscript/antlr/Wurst.g4 @@ -204,7 +204,7 @@ modifier: 'public' | 'private' | 'protected' - | 'publicread' + | 'readonly' | 'static' | 'override' | 'abstract' @@ -491,7 +491,7 @@ ENDPACKAGE: 'endpackage'; FUNCTION: 'function'; RETURNS: 'returns'; PUBLIC: 'public'; -PULBICREAD: 'publicread'; +READONLY: 'readonly'; PRIVATE: 'private'; PROTECTED: 'protected'; IMPORT: 'import'; diff --git a/de.peeeq.wurstscript/src/main/antlr/de/peeeq/wurstscript/jurst/antlr/Jurst.g4 b/de.peeeq.wurstscript/src/main/antlr/de/peeeq/wurstscript/jurst/antlr/Jurst.g4 index 8f1c6ca06..6d53d6a2d 100644 --- a/de.peeeq.wurstscript/src/main/antlr/de/peeeq/wurstscript/jurst/antlr/Jurst.g4 +++ b/de.peeeq.wurstscript/src/main/antlr/de/peeeq/wurstscript/jurst/antlr/Jurst.g4 @@ -203,7 +203,6 @@ modifier: 'public' | 'private' | 'protected' - | 'publicread' | 'readonly' | 'static' | 'override' @@ -488,7 +487,6 @@ ENDPACKAGE: 'endpackage'; FUNCTION: 'function'; RETURNS: 'returns'; PUBLIC: 'public'; -PULBICREAD: 'publicread'; READONLY: 'readonly'; DELEGATE: 'delegate'; STUB: 'stub'; diff --git a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstio/languageserver/requests/HoverInfo.java b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstio/languageserver/requests/HoverInfo.java index 6af702584..edc94658f 100644 --- a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstio/languageserver/requests/HoverInfo.java +++ b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstio/languageserver/requests/HoverInfo.java @@ -804,8 +804,8 @@ public List> case_Indexes(Indexes indexes) { } @Override - public List> case_VisibilityPublicread(VisibilityPublicread visibilityPublicread) { - return string("This variable can be read from everywhere but only written to in this scope."); + public List> case_ModReadonly(ModReadonly readonly) { + return string("This variable can only be written from its declaring scope."); } @Override diff --git a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/attributes/DescriptionHtml.java b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/attributes/DescriptionHtml.java index db1d39500..c30c5a104 100644 --- a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/attributes/DescriptionHtml.java +++ b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/attributes/DescriptionHtml.java @@ -379,8 +379,8 @@ public static String description(VisibilityPublic visibilityPublic) { return "public: can be used in other packages"; } - public static @Nullable String description(VisibilityPublicread visibilityPublicread) { - return null; + public static String description(ModReadonly readonly) { + return "readonly: can only be written from its declaring scope"; } public static @Nullable String description(WBlock wBlock) { diff --git a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/attributes/ModifiersHelper.java b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/attributes/ModifiersHelper.java index f78b375ed..b75ca302d 100644 --- a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/attributes/ModifiersHelper.java +++ b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/attributes/ModifiersHelper.java @@ -14,8 +14,8 @@ public static boolean isProtected(HasModifier e) { return containsType(e.getModifiers(), VisibilityProtected.class); } - public static boolean isPublicRead(HasModifier e) { - return containsType(e.getModifiers(), VisibilityPublicread.class); + public static boolean isReadonly(HasModifier e) { + return containsType(e.getModifiers(), ModReadonly.class); } public static boolean isPrivate(HasModifier e) { diff --git a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/attributes/prettyPrint/PrettyPrinter.java b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/attributes/prettyPrint/PrettyPrinter.java index 995cc37db..a0a477b11 100644 --- a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/attributes/prettyPrint/PrettyPrinter.java +++ b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/attributes/prettyPrint/PrettyPrinter.java @@ -1295,8 +1295,8 @@ public static void prettyPrint(VisibilityPublic e, Spacer spacer, StringBuilder sb.append("public"); } - public static void prettyPrint(VisibilityPublicread e, Spacer spacer, StringBuilder sb, int indent) { - sb.append("publicread"); + public static void prettyPrint(ModReadonly e, Spacer spacer, StringBuilder sb, int indent) { + sb.append("readonly"); } public static void prettyPrint(WBlock wBlock, Spacer spacer, StringBuilder sb, int indent) { diff --git a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/jurst/AntlrJurstParseTreeTransformer.java b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/jurst/AntlrJurstParseTreeTransformer.java index 9bdc534d4..dca9f2e59 100644 --- a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/jurst/AntlrJurstParseTreeTransformer.java +++ b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/jurst/AntlrJurstParseTreeTransformer.java @@ -349,9 +349,8 @@ private Modifier transformModifier(ModifierContext m) { return Ast.VisibilityPrivate(src); case JurstParser.PROTECTED: return Ast.VisibilityProtected(src); - case JurstParser.PULBICREAD: case JurstParser.READONLY: - return Ast.VisibilityPublicread(src); + return Ast.ModReadonly(src); case JurstParser.STATIC: return Ast.ModStatic(src); case JurstParser.OVERRIDE: diff --git a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/parser/antlr/AntlrWurstParseTreeTransformer.java b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/parser/antlr/AntlrWurstParseTreeTransformer.java index a2ead963f..ee2565781 100644 --- a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/parser/antlr/AntlrWurstParseTreeTransformer.java +++ b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/parser/antlr/AntlrWurstParseTreeTransformer.java @@ -433,8 +433,8 @@ private Modifier transformModifier(ModifierContext m) { return Ast.VisibilityPrivate(src); case WurstParser.PROTECTED: return Ast.VisibilityProtected(src); - case WurstParser.PULBICREAD: - return Ast.VisibilityPublicread(src); + case WurstParser.READONLY: + return Ast.ModReadonly(src); case WurstParser.STATIC: return Ast.ModStatic(src); case WurstParser.OVERRIDE: diff --git a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/validation/WurstValidator.java b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/validation/WurstValidator.java index 985dc3a48..d5d4dc070 100644 --- a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/validation/WurstValidator.java +++ b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/validation/WurstValidator.java @@ -1240,7 +1240,7 @@ private void checkStmtSet(StmtSet s) { return; } checkAssignment(Utils.isJassCode(s), s, setOverload.getParameterType(1), rightType); - checkIfAssigningToConstant(s.getUpdatedExpr()); + checkIfAssigningToRestrictedVariable(s.getUpdatedExpr()); checkIfNoEffectAssignment(s); return; } @@ -1248,7 +1248,7 @@ private void checkStmtSet(StmtSet s) { WurstType leftType = s.getUpdatedExpr().attrTyp(); checkAssignment(Utils.isJassCode(s), s, leftType, rightType); - checkIfAssigningToConstant(s.getUpdatedExpr()); + checkIfAssigningToRestrictedVariable(s.getUpdatedExpr()); checkIfNoEffectAssignment(s); } @@ -1294,17 +1294,18 @@ && refersToSameVar(va.attrImplicitParameter(), vb.attrImplicitParameter())) { return false; } - private void checkIfAssigningToConstant(final LExpr left) { + private void checkIfAssigningToRestrictedVariable(final LExpr left) { left.match(new LExpr.MatcherVoid() { @Override public void case_ExprVarArrayAccess(ExprVarArrayAccess e) { - + checkVarNotReadonly(e, e.attrNameLink()); } @Override public void case_ExprVarAccess(ExprVarAccess e) { checkVarNotConstant(e, e.attrNameLink()); + checkVarNotReadonly(e, e.attrNameLink()); } @Override @@ -1315,18 +1316,19 @@ public void case_ExprMemberVarDot(ExprMemberVarDot e) { if (e.getLeft() instanceof ExprThis) { e.addError("Cannot change 'this'. Tuples are not classes."); } else if (e.getLeft() instanceof NameRef) { - checkIfAssigningToConstant((NameRef) e.getLeft()); + checkIfAssigningToRestrictedVariable((NameRef) e.getLeft()); } else { e.addError( "Ok, so you are trying to assign something to the return value of a function. This wont do nothing. Tuples are not classes."); } } checkVarNotConstant(e, e.attrNameLink()); + checkVarNotReadonly(e, e.attrNameLink()); } @Override public void case_ExprMemberArrayVarDot(ExprMemberArrayVarDot e) { - + checkVarNotReadonly(e, e.attrNameLink()); } @Override @@ -1347,6 +1349,47 @@ public void case_ExprMemberVarQuestionDot(ExprMemberVarQuestionDot e) { }); } + private void checkVarNotReadonly(NameRef left, @Nullable NameLink link) { + if (link == null || !(link.getDef() instanceof GlobalVarDef)) { + return; + } + GlobalVarDef var = (GlobalVarDef) link.getDef(); + if (!var.attrIsReadonly()) { + return; + } + + ModuleInstanciation moduleOwner = nearestModuleInstanciation(var); + if (moduleOwner != null) { + if (nearestModuleInstanciation(left) != moduleOwner) { + left.addError("Readonly member " + var.getName() + + " can only be assigned from its declaring module."); + } + return; + } + + ClassOrModule owner = var.attrNearestClassOrModule(); + if (owner != null) { + if (left.attrNearestClassOrModule() != owner) { + left.addError("Readonly member " + var.getName() + + " can only be assigned from its declaring " + + (owner instanceof ClassDef ? "class" : "module") + "."); + } + } else if (left.attrNearestPackage() != var.attrNearestPackage()) { + left.addError("Readonly variable " + var.getName() + + " can only be assigned from its declaring package."); + } + } + + private @Nullable ModuleInstanciation nearestModuleInstanciation(Element element) { + while (element != null) { + if (element instanceof ModuleInstanciation) { + return (ModuleInstanciation) element; + } + element = element.getParent(); + } + return null; + } + private void checkVarNotConstant(NameRef left, @Nullable NameLink link) { if (link == null) { return; @@ -2589,10 +2632,10 @@ public void case_LocalVarDef(LocalVarDef localVarDef) { @Override public void case_GlobalVarDef(GlobalVarDef g) { if (g.attrNearestClassOrModule() != null) { - check(VisibilityPrivate.class, VisibilityProtected.class, - ModStatic.class, ModConstant.class, Annotation.class); + check(VisibilityPublic.class, VisibilityPrivate.class, VisibilityProtected.class, + ModStatic.class, ModConstant.class, ModReadonly.class, Annotation.class); } else { - check(VisibilityPublic.class, ModConstant.class, Annotation.class); + check(VisibilityPublic.class, ModConstant.class, ModReadonly.class, Annotation.class); } if (g.hasAnnotation("@compiletime")) { g.getAnnotation("@compiletime") diff --git a/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/ClassesTests.java b/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/ClassesTests.java index 5d76453b9..b61b4c7f9 100644 --- a/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/ClassesTests.java +++ b/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/ClassesTests.java @@ -1854,6 +1854,174 @@ public void initialization_static_inner2() { ); } + @Test + public void readonlyMemberCanBeReadExternallyAndWrittenByOwner() { + test().testLua(true).executeProg().lines( + "package test", + "native testSuccess()", + "class Counter", + " public readonly int value", + " function setValue(int newValue)", + " value = newValue", + "init", + " let counter = new Counter()", + " counter.setValue(1)", + " counter.setValue(42)", + " if counter.value == 42", + " testSuccess()" + ); + } + + @Test + public void readonlyMemberRejectsExternalAssignment() { + testAssertErrorsLines(false, "Readonly member value can only be assigned from its declaring class", + "package test", + "class Counter", + " public readonly int value", + "init", + " let counter = new Counter()", + " counter.value = 42" + ); + } + + @Test + public void readonlyMemberRejectsExternalIncrement() { + testAssertErrorsLines(false, "Readonly member value can only be assigned from its declaring class", + "package test", + "class Counter", + " public readonly int value", + "init", + " let counter = new Counter()", + " counter.value++" + ); + } + + @Test + public void readonlyMemberRejectsSubclassAssignment() { + testAssertErrorsLines(false, "Readonly member value can only be assigned from its declaring class", + "package test", + "class Parent", + " public readonly int value", + "class Child extends Parent", + " function mutate()", + " value = 42", + "init", + " new Child().mutate()" + ); + } + + @Test + public void readonlyStaticArrayCanOnlyBeWrittenByOwner() { + test().testLua(true).executeProg().lines( + "package test", + "native testSuccess()", + "class Values", + " public static readonly int array entries", + " static function put(int index, int value)", + " entries[index] = value", + "init", + " Values.put(3, 42)", + " if Values.entries[3] == 42", + " testSuccess()" + ); + } + + @Test + public void readonlyStaticArrayRejectsExternalElementAssignment() { + testAssertErrorsLines(false, "Readonly member entries can only be assigned from its declaring class", + "package test", + "class Values", + " public static readonly int array entries", + "init", + " Values.entries[3] = 42" + ); + } + + @Test + public void publicreadIsNoLongerReserved() { + testAssertOkLines(true, + "package test", + "native testSuccess()", + "init", + " let publicread = 42", + " if publicread == 42", + " testSuccess()" + ); + } + + @Test + public void readonlyPackageVariableCanBeReadExternallyAndWrittenByOwner() { + test().executeProg().compilationUnits( + compilationUnit("owner.wurst", + "package owner", + "public readonly int value", + "public function setValue(int newValue)", + " value = newValue", + "endpackage"), + compilationUnit("consumer.wurst", + "package consumer", + "import owner", + "native testSuccess()", + "init", + " setValue(42)", + " if value == 42", + " testSuccess()", + "endpackage") + ); + } + + @Test + public void readonlyPackageVariableRejectsExternalAssignment() { + test().executeProg(false) + .expectError("Readonly variable value can only be assigned from its declaring package") + .compilationUnits( + compilationUnit("owner.wurst", + "package owner", + "public readonly int value", + "endpackage"), + compilationUnit("consumer.wurst", + "package consumer", + "import owner", + "init", + " value = 42", + "endpackage") + ); + } + + @Test + public void readonlyModuleMemberCanBeWrittenByModuleMethod() { + test().testLua(true).executeProg().lines( + "package test", + "native testSuccess()", + "module Values", + " public readonly int value", + " function setValue(int newValue)", + " value = newValue", + "class C", + " use Values", + "init", + " let c = new C()", + " c.setValue(42)", + " if c.value == 42", + " testSuccess()" + ); + } + + @Test + public void readonlyModuleMemberRejectsConsumingClassAssignment() { + testAssertErrorsLines(false, "Readonly member value can only be assigned from its declaring module", + "package test", + "module Values", + " public readonly int value", + "class C", + " use Values", + " function mutate()", + " value = 42", + "init", + " new C().mutate()" + ); + } +