Skip to content

Commit

Permalink
Give an error on duplicate argument names in transactions.
Browse files Browse the repository at this point in the history
  • Loading branch information
mcoblenz committed Jan 28, 2020
1 parent c9e7c75 commit bd93470
Show file tree
Hide file tree
Showing 5 changed files with 55 additions and 3 deletions.
2 changes: 1 addition & 1 deletion resources/demos/ERC20/ERC20.obs
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ asset interface ERC20 {
transaction approve(int ownerAddress, int fromAddress, int value) returns bool;

// Returns the amount of allowance still available.
transaction allowance(int ownerAddress, int ownerAddress) returns int;
transaction allowance(int ownerAddress, int fromAddress) returns int;

// Transfers tokens from an allowance that has already been granted.
transaction transferFrom(int senderAddress, int fromAddress, int toAddress, int value) returns bool;
Expand Down
9 changes: 9 additions & 0 deletions resources/tests/type_checker_tests/DuplicateArgs.obs
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
main contract Names {
// Shouldn't have duplicate names
transaction duplicateNames(int y, int y) {
}
}

interface Duplicates {
transaction duplicateNames(int y, int y);
}
30 changes: 29 additions & 1 deletion src/main/scala/edu/cmu/cs/obsidian/typecheck/Checker.scala
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ import edu.cmu.cs.obsidian.parser.Parser.Identifier
import edu.cmu.cs.obsidian.parser._

import scala.collection.immutable.TreeMap
import scala.collection.mutable
import scala.collection.mutable.HashSet


/* We define a custom type to store a special flag for if a context in after a "throw".
Expand Down Expand Up @@ -2310,6 +2312,16 @@ class Checker(globalTable: SymbolTable, verbose: Boolean = false) {
field
}

private def checkDuplicateArgNames(tx: Transaction): Unit = {
val argNames = mutable.HashSet[String]()
for (arg <- tx.args) {
if (argNames.exists(arg.varName.equals)) {
logError(tx, DuplicateArgName(arg.varName))
}
argNames.add(arg.varName)
}
}

private def checkTransactionInState(tx: Transaction,
lexicallyInsideOf: DeclarationTable,
initContext: Context): Transaction = {
Expand All @@ -2331,6 +2343,7 @@ class Checker(globalTable: SymbolTable, verbose: Boolean = false) {
}
}

checkDuplicateArgNames(tx);
var context = initContext

for (arg <- tx.args) {
Expand Down Expand Up @@ -2823,6 +2836,18 @@ class Checker(globalTable: SymbolTable, verbose: Boolean = false) {
}
}

private def checkInterfaceDeclaration(decl: Declaration): Unit = {
decl match {
case t: Transaction =>
checkDuplicateArgNames(t)
case s: State => ()
case f: Field => logError(f, FieldsNotAllowedInInterfaces(f.name, decl.name))
case c: Constructor => logError(c, InterfaceConstructorError())
case c: Contract => assert(false, "Contract nesting is no longer supported.")
case d: TypeDecl => assert(false, "Type declarations are not supported yet.")
}
}

private def checkContract(contract: Contract): Contract = {
currentContractSourcePath = contract.sourcePath

Expand All @@ -2840,7 +2865,10 @@ class Checker(globalTable: SymbolTable, verbose: Boolean = false) {

var newDecls = obsContract.declarations

if (!obsContract.isInterface) {
if (obsContract.isInterface) {
obsContract.declarations.map(checkInterfaceDeclaration)
}
else {
val boundDecls = globalTable.contract(obsContract.bound.contractName) match {
case Some(interface) => interface.contract match {
case impl: ObsidianContractImpl => impl.substitute(impl.params, obsContract.bound.typeArgs).declarations
Expand Down
10 changes: 9 additions & 1 deletion src/main/scala/edu/cmu/cs/obsidian/typecheck/Error.scala
Original file line number Diff line number Diff line change
Expand Up @@ -457,7 +457,7 @@ case class PermissionCheckRedundant(actualPerm: Permission, testPermission: Perm

case class AssetStateImplError(implState: State, interfaceState: State) extends Error {
override val msg: String =
s"State ${implState.name} is an asset state but it implements a state which is not an asset state."
s"State ${implState.name} is an asset state but it implements a state that is not an asset state."
}

case class BadFFIInterfaceBoundError(name: String) extends Error {
Expand All @@ -478,4 +478,12 @@ case class StateInitializerUninitialized(stateName: String, fieldName: String) e
override val msg: String = s"$stateName::$fieldName has not been initialized. Perhaps you are " +
s"trying to use a field initializer for general-purpose field access instead of only for " +
s"preparing for a transition."
}

case class DuplicateArgName(argName: String) extends Error {
override val msg: String = s"All argument names must be different from each other, but '$argName' occurs more than once."
}

case class FieldsNotAllowedInInterfaces(fieldName: String, interfaceName: String) extends Error {
override val msg: String = s"Field '$fieldName' cannot be defined on interface '$interfaceName' because interfaces cannot define fields."
}
Original file line number Diff line number Diff line change
Expand Up @@ -977,4 +977,11 @@ class TypeCheckerTests extends JUnitSuite {
runTest("resources/tests/type_checker_tests/InStateOwnershipChange.obs",
(InvalidOwnershipLossInDynamicCheck("k"), 11) :: Nil)
}

@Test def duplicateNames(): Unit = {
runTest("resources/tests/type_checker_tests/DuplicateArgs.obs",
(DuplicateArgName("y"), 3) ::
(DuplicateArgName("y"), 8) ::
Nil)
}
}

0 comments on commit bd93470

Please sign in to comment.