Skip to content

Commit

Permalink
GROOVY-7632: Groovy named parameters static check (initial cut of sup…
Browse files Browse the repository at this point in the history
…port) (closes #822)
  • Loading branch information
paulk-asert committed Nov 16, 2018
1 parent 3ac1abc commit 3bdea65
Show file tree
Hide file tree
Showing 4 changed files with 212 additions and 2 deletions.
Expand Up @@ -22,6 +22,8 @@
import groovy.lang.DelegatesTo;
import groovy.lang.IntRange;
import groovy.lang.Range;
import groovy.transform.NamedParam;
import groovy.transform.NamedParams;
import groovy.transform.TypeChecked;
import groovy.transform.TypeCheckingMode;
import groovy.transform.stc.ClosureParams;
Expand All @@ -43,6 +45,7 @@
import org.codehaus.groovy.ast.Parameter;
import org.codehaus.groovy.ast.PropertyNode;
import org.codehaus.groovy.ast.Variable;
import org.codehaus.groovy.ast.expr.AnnotationConstantExpression;
import org.codehaus.groovy.ast.expr.ArgumentListExpression;
import org.codehaus.groovy.ast.expr.AttributeExpression;
import org.codehaus.groovy.ast.expr.BinaryExpression;
Expand Down Expand Up @@ -285,6 +288,7 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
protected static final ClassNode DELEGATES_TO_TARGET = ClassHelper.make(DelegatesTo.Target.class);
protected static final ClassNode LINKEDHASHMAP_CLASSNODE = make(LinkedHashMap.class);
protected static final ClassNode CLOSUREPARAMS_CLASSNODE = make(ClosureParams.class);
protected static final ClassNode NAMED_PARAMS_CLASSNODE = make(NamedParams.class);
protected static final ClassNode MAP_ENTRY_TYPE = make(Map.Entry.class);
protected static final ClassNode ENUMERATION_TYPE = make(Enumeration.class);

Expand Down Expand Up @@ -2609,6 +2613,86 @@ protected void visitMethodCallArguments(final ClassNode receiver, ArgumentListEx
}
}
}
if (expressions.size() > 0 && expressions.get(0) instanceof MapExpression && params.length > 0) {
checkNamedParamsAnnotation(params[0], (MapExpression) expressions.get(0));
}
}

private void checkNamedParamsAnnotation(Parameter param, MapExpression args) {
if (!param.getType().isDerivedFrom(ClassHelper.MAP_TYPE)) return;
List<MapEntryExpression> entryExpressions = args.getMapEntryExpressions();
Map<Object, Expression> entries = new LinkedHashMap<Object, Expression>();
for (MapEntryExpression entry : entryExpressions) {
Object key = entry.getKeyExpression();
if (key instanceof ConstantExpression) {
key = ((ConstantExpression) key).getValue();
}
entries.put(key, entry.getValueExpression());
}
List<AnnotationNode> annotations = param.getAnnotations(NAMED_PARAMS_CLASSNODE);
if (annotations != null && !annotations.isEmpty()) {
AnnotationNode an = null;
for (AnnotationNode next : annotations) {
if (next.getClassNode().getName().equals(NamedParams.class.getName())) {
an = next;
}
}
List<String> collectedNames = new ArrayList<String>();
if (an != null) {
Expression value = an.getMember("value");
if (value instanceof AnnotationConstantExpression) {
processNamedParam((AnnotationConstantExpression) value, entries, args, collectedNames);
} else if (value instanceof ListExpression) {
ListExpression le = (ListExpression) value;
for (Expression next : le.getExpressions()) {
if (next instanceof AnnotationConstantExpression) {
processNamedParam((AnnotationConstantExpression) next, entries, args, collectedNames);
}
}
}
for (Map.Entry<Object, Expression> entry : entries.entrySet()) {
if (!collectedNames.contains(entry.getKey())) {
addStaticTypeError("unexpected named arg: " + entry.getKey(), args);
}
}
}
}
}

private void processNamedParam(AnnotationConstantExpression value, Map<Object, Expression> entries, Expression expression, List<String> collectedNames) {
AnnotationNode namedParam = (AnnotationNode) value.getValue();
if (!namedParam.getClassNode().getName().equals(NamedParam.class.getName())) return;
String name = null;
boolean required = false;
ClassNode expectedType = null;
ConstantExpression constX = (ConstantExpression) namedParam.getMember("value");
if (constX != null) {
name = (String) constX.getValue();
collectedNames.add(name);
}
constX = (ConstantExpression) namedParam.getMember("required");
if (constX != null) {
required = (Boolean) constX.getValue();
}
ClassExpression typeX = (ClassExpression) namedParam.getMember("type");
if (typeX != null) {
expectedType = typeX.getType();
}
if (!entries.keySet().contains(name)) {
if (required) {
addStaticTypeError("required named arg '" + name + "' not found.", expression);
}
} else {
Expression supplied = entries.get(name);
if (isCompatibleType(expectedType, expectedType != null, supplied.getType())) {
addStaticTypeError("parameter for named arg '" + name + "' has type '" + prettyPrintType(supplied.getType()) +
"' but expected '" + prettyPrintType(expectedType) + "'.", expression);
}
}
}

private boolean isCompatibleType(ClassNode expectedType, boolean b, ClassNode type) {
return b && !isAssignableTo(type, expectedType);
}

/**
Expand Down
40 changes: 40 additions & 0 deletions src/test/groovy/NamedParameterHelper.java
@@ -0,0 +1,40 @@
/*
* Licensed to the Apache Software Foundation (ASF) under one
* or more contributor license agreements. See the NOTICE file
* distributed with this work for additional information
* regarding copyright ownership. The ASF licenses this file
* to you 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
*
* http://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 groovy;

import groovy.transform.NamedParam;
import groovy.transform.NamedParams;

import java.util.LinkedHashMap;

public class NamedParameterHelper {
public static String myJavaMethod(@NamedParams({
@NamedParam(value = "foo"),
@NamedParam(value = "bar", type = String.class, required = true)
}) LinkedHashMap params) {
return "foo = " + params.get("foo") + ", bar = " + params.get("bar");
}

public static String myJavaMethod(@NamedParams({
@NamedParam(value = "foo", type = String.class, required = true),
@NamedParam(value = "bar", type = Integer.class)
}) LinkedHashMap params, int num) {
return "foo = " + params.get("foo") + ", bar = " + params.get("bar") + ", num = " + num;
}
}
87 changes: 87 additions & 0 deletions src/test/groovy/NamedParameterTest.groovy
Expand Up @@ -18,6 +18,12 @@
*/
package groovy

import groovy.transform.NamedParam
import groovy.transform.NamedParams
import groovy.transform.TypeChecked

import static groovy.NamedParameterHelper.myJavaMethod

class NamedParameterTest extends GroovyTestCase {

void testPassingNamedParametersToMethod() {
Expand Down Expand Up @@ -48,4 +54,85 @@ class NamedParameterTest extends GroovyTestCase {
times:
2
}

@TypeChecked
void testNamedParamsAnnotation() {
assert myJavaMethod(foo: 'FOO', bar: 'BAR') == 'foo = FOO, bar = BAR'
assert myJavaMethod(bar: 'BAR') == 'foo = null, bar = BAR'
assert myJavaMethod(foo: 'FOO', bar: 25, 42) == 'foo = FOO, bar = 25, num = 42'
assert myJavaMethod(foo: 'FOO', 142) == 'foo = FOO, bar = null, num = 142'
assert myMethod(foo: 'FOO', bar: 'BAR') == 'foo = FOO, bar = BAR'
assert myMethod(bar: 'BAR') == 'foo = null, bar = BAR'
assert myMethod(foo: 'FOO', bar: 35,242) == 'foo = FOO, bar = 35, num = 242'
assert myMethod(foo: 'FOO', 342) == 'foo = FOO, bar = null, num = 342'
assertScript '''
import groovy.transform.TypeChecked
import static groovy.NamedParameterTest.myMethod
@TypeChecked
def method() {
assert myMethod(foo: 'FOO', bar: 'BAR') == 'foo = FOO, bar = BAR'
assert myMethod(bar: 'BAR') == 'foo = null, bar = BAR'
assert myMethod(foo: 'FOO', bar: 45, 442) == 'foo = FOO, bar = 45, num = 442'
assert myMethod(foo: 'FOO', 542) == 'foo = FOO, bar = null, num = 542'
}
method()
'''
}

void testMissingRequiredName() {
def message = shouldFail '''
import groovy.transform.TypeChecked
import static groovy.NamedParameterTest.myMethod
@TypeChecked
def method() {
myMethod(foo: 'FOO')
}
method()
'''
assert message.contains("required named arg 'bar' not found")
}

void testUnknownName() {
def message = shouldFail '''
import groovy.transform.TypeChecked
import static groovy.NamedParameterTest.myMethod
@TypeChecked
def method() {
myMethod(bar: 'BAR', baz: 'BAZ')
}
method()
'''
assert message.contains("unexpected named arg: baz")
}

void testInvalidType() {
def message = shouldFail '''
import groovy.transform.TypeChecked
import static groovy.NamedParameterTest.myMethod
@TypeChecked
def method() {
myMethod(foo: 42, 42)
}
method()
'''
assert message.contains("parameter for named arg 'foo' has type 'int' but expected 'java.lang.String'")
}

static String myMethod(@NamedParams([
@NamedParam(value = "foo"),
@NamedParam(value = "bar", type = String, required = true)
]) Map params) {
"foo = $params.foo, bar = $params.bar"
}

static String myMethod(@NamedParams([
@NamedParam(value = "foo", type = String, required = true),
@NamedParam(value = "bar", type = Integer)
]) Map params, int num) {
"foo = $params.foo, bar = $params.bar, num = $num"
}
}
Expand Up @@ -2632,8 +2632,7 @@ public Expression visitEnhancedArgumentList(EnhancedArgumentListContext ctx) {

if (asBoolean(mapEntryExpressionList) && asBoolean(expressionList)) { // e.g. arguments like x: 1, 'a', y: 2, 'b', z: 3
ArgumentListExpression argumentListExpression = new ArgumentListExpression(expressionList);
argumentListExpression.getExpressions().add(0, new MapExpression(mapEntryExpressionList)); // TODO: confirm BUG OR NOT? All map entries will be put at first, which is not friendly to Groovy developers

argumentListExpression.getExpressions().add(0, configureAST(new MapExpression(mapEntryExpressionList), ctx));
return configureAST(argumentListExpression, ctx);
}

Expand Down

0 comments on commit 3bdea65

Please sign in to comment.