Skip to content

Commit

Permalink
Merge pull request hyperledger-web3j#436 from mushketyk/transaction-r…
Browse files Browse the repository at this point in the history
…eturn-warning

Outputs a warning when non-constant function returns a value
  • Loading branch information
conor10 authored Apr 3, 2018
2 parents fc1d053 + 00cc19f commit 6081d50
Show file tree
Hide file tree
Showing 5 changed files with 78 additions and 2 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
package org.web3j.codegen;

/**
* Can be used to provide report about a code generation process.
*/
interface GenerationReporter {
void report(String msg);
}
20 changes: 20 additions & 0 deletions codegen/src/main/java/org/web3j/codegen/LogGenerationReporter.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
package org.web3j.codegen;

import org.slf4j.Logger;

/**
* A reporter generation that outputs messages using a logger instance.
*/
class LogGenerationReporter implements GenerationReporter {

private final Logger logger;

public LogGenerationReporter(Logger logger) {
this.logger = logger;
}

@Override
public void report(String msg) {
logger.warn(msg);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@
import com.squareup.javapoet.TypeName;
import com.squareup.javapoet.TypeSpec;
import com.squareup.javapoet.TypeVariableName;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import rx.functions.Func1;

import org.web3j.abi.EventEncoder;
Expand Down Expand Up @@ -73,6 +75,7 @@ public class SolidityFunctionWrapper extends Generator {
private static final String WEI_VALUE = "weiValue";

private static final ClassName LOG = ClassName.get(Log.class);
private static final Logger LOGGER = LoggerFactory.getLogger(SolidityFunctionWrapper.class);

private static final String CODEGEN_WARNING = "<p>Auto generated code.\n"
+ "<p><strong>Do not modify!</strong>\n"
Expand All @@ -85,9 +88,15 @@ public class SolidityFunctionWrapper extends Generator {
private final boolean useNativeJavaTypes;
private static final String regex = "(\\w+)(?:\\[(.*?)\\])(?:\\[(.*?)\\])?";
private static final Pattern pattern = Pattern.compile(regex);
private final GenerationReporter reporter;

public SolidityFunctionWrapper(boolean useNativeJavaTypes) {
this(useNativeJavaTypes, new LogGenerationReporter(LOGGER));
}

SolidityFunctionWrapper(boolean useNativeJavaTypes, GenerationReporter reporter) {
this.useNativeJavaTypes = useNativeJavaTypes;
this.reporter = reporter;
}

@SuppressWarnings("unchecked")
Expand Down Expand Up @@ -659,11 +668,20 @@ private static ParameterizedTypeName buildRemoteCall(TypeName typeName) {
ClassName.get(RemoteCall.class), typeName);
}

private static void buildTransactionFunction(
private void buildTransactionFunction(
AbiDefinition functionDefinition,
MethodSpec.Builder methodBuilder,
String inputParams) throws ClassNotFoundException {

if (functionDefinition.hasOutputs()) {
//CHECKSTYLE:OFF
reporter.report(String.format(
"Definition of the function %s returns a value but is not defined as a view function. "
+ "Please ensure it contains the view modifier if you want to read the return value",
functionDefinition.getName()));
//CHECKSTYLE:ON
}

if (functionDefinition.isPayable()) {
methodBuilder.addParameter(BigInteger.class, WEI_VALUE);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,8 @@
import static org.hamcrest.core.Is.is;
import static org.hamcrest.core.IsEqual.equalTo;
import static org.junit.Assert.assertThat;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.verify;
import static org.web3j.codegen.SolidityFunctionWrapper.buildTypeName;
import static org.web3j.codegen.SolidityFunctionWrapper.createValidParamName;
import static org.web3j.codegen.SolidityFunctionWrapper.getEventNativeType;
Expand All @@ -42,10 +44,13 @@ public class SolidityFunctionWrapperTest extends TempFileProvider {

private SolidityFunctionWrapper solidityFunctionWrapper;

private GenerationReporter generationReporter;

@Override
public void setUp() throws Exception {
super.setUp();
solidityFunctionWrapper = new SolidityFunctionWrapper(true);
generationReporter = mock(GenerationReporter.class);
solidityFunctionWrapper = new SolidityFunctionWrapper(true, generationReporter);
}

@Test
Expand Down Expand Up @@ -168,6 +173,27 @@ public void testBuildFunctionTransaction() throws Exception {
assertThat(methodSpec.toString(), is(expected));
}

@Test
public void testBuildingFunctionTransactionThatReturnsValueReportsWarning() throws Exception {
AbiDefinition functionDefinition = new AbiDefinition(
false,
Arrays.asList(
new AbiDefinition.NamedType("param", "uint8")),
"functionName",
Arrays.asList(
new AbiDefinition.NamedType("result", "uint8")),
"type",
false);

solidityFunctionWrapper.buildFunction(functionDefinition);

//CHECKSTYLE:OFF
verify(generationReporter).report(
"Definition of the function functionName returns a value but is not defined as a view function. " +
"Please ensure it contains the view modifier if you want to read the return value");
//CHECKSTYLE:ON
}

@Test
public void testBuildPayableFunctionTransaction() throws Exception {
AbiDefinition functionDefinition = new AbiDefinition(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,10 @@ public List<NamedType> getOutputs() {
return outputs;
}

public boolean hasOutputs() {
return !outputs.isEmpty();
}

public void setOutputs(List<NamedType> outputs) {
this.outputs = outputs;
}
Expand Down

0 comments on commit 6081d50

Please sign in to comment.