Merge pull request #1968 from yann300/bugFix

Mix - Secondary locations for errors.

mix/CodeModel.cpp

@@ -47,7 +47,7 @@
 using namespace dev::mix;
 
 const std::set<std::string> c_predefinedContracts =
-	{ "Config", "Coin", "CoinReg", "coin", "service", "owned", "mortal", "NameReg", "named", "std", "configUser" };
+{ "Config", "Coin", "CoinReg", "coin", "service", "owned", "mortal", "NameReg", "named", "std", "configUser" };
 
 
 namespace
@@ -59,7 +59,7 @@ class CollectLocalsVisitor: public ASTConstVisitor
 {
 public:
 	CollectLocalsVisitor(QHash<LocationPair, SolidityDeclaration>* _locals):
-	m_locals(_locals), m_functionScope(false) {}
+		m_locals(_locals), m_functionScope(false) {}
 
 private:
 	LocationPair nodeLocation(ASTNode const& _node)
@@ -99,7 +99,7 @@ class CollectLocationsVisitor: public ASTConstVisitor
 {
 public:
 	CollectLocationsVisitor(SourceMap* _sourceMap):
-	m_sourceMap(_sourceMap) {}
+		m_sourceMap(_sourceMap) {}
 
 private:
 	LocationPair nodeLocation(ASTNode const& _node)
@@ -313,24 +313,47 @@ void CodeModel::runCompilationJob(int _jobId)
 	}
 	catch (dev::Exception const& _exception)
 	{
-		std::ostringstream error;
+		std::stringstream error;
 		solidity::SourceReferenceFormatter::printExceptionInformation(error, _exception, "Error", cs);
 		QString message = QString::fromStdString(error.str());
-		QString sourceName;
+		QVariantMap firstLocation;
-		if (SourceLocation const* location = boost::get_error_info<solidity::errinfo_sourceLocation>(_exception))
+		QVariantList secondLocations;
+		if (SourceLocation const* first = boost::get_error_info<solidity::errinfo_sourceLocation>(_exception))
+			firstLocation = resolveCompilationErrorLocation(cs, *first);
+		if (SecondarySourceLocation const* second = boost::get_error_info<solidity::errinfo_secondarySourceLocation>(_exception))
 		{
-			if (location->sourceName)
+			for (auto const& c: second->infos)
-				sourceName = QString::fromStdString(*location->sourceName);
+				secondLocations.push_back(resolveCompilationErrorLocation(cs, c.second));
-			if (!sourceName.isEmpty())
-				if (CompiledContract* contract = contractByDocumentId(sourceName))
-					message = message.replace(sourceName, contract->contract()->name()); //substitute the location to match our contract names
 		}
-		compilationError(message, sourceName);
+		compilationError(message, firstLocation, secondLocations);
 	}
 	m_compiling = false;
 	emit stateChanged();
 }
 
+QVariantMap CodeModel::resolveCompilationErrorLocation(CompilerStack const& _compiler, SourceLocation const& _location)
+{
+	std::tuple<int, int, int, int> pos = _compiler.positionFromSourceLocation(_location);
+	QVariantMap startError;
+	startError.insert("line", std::get<0>(pos) > 1 ? (std::get<0>(pos) - 1) : 1);
+	startError.insert("column", std::get<1>(pos) > 1 ? (std::get<1>(pos) - 1) : 1);
+	QVariantMap endError;
+	endError.insert("line", std::get<2>(pos) > 1 ? (std::get<2>(pos) - 1) : 1);
+	endError.insert("column", std::get<3>(pos) > 1 ? (std::get<3>(pos) - 1) : 1);
+	QVariantMap error;
+	error.insert("start", startError);
+	error.insert("end", endError);
+	QString sourceName;
+	if (_location.sourceName)
+		sourceName = QString::fromStdString(*_location.sourceName);
+	error.insert("source", sourceName);
+	if (!sourceName.isEmpty())
+		if (CompiledContract* contract = contractByDocumentId(sourceName))
+			sourceName = contract->contract()->name(); //substitute the location to match our contract names
+	error.insert("contractName", sourceName);
+	return error;
+}
+
 void CodeModel::gasEstimation(solidity::CompilerStack const& _cs)
 {
 	if (m_gasCostsMaps)
@@ -411,7 +434,7 @@ void CodeModel::collectContracts(dev::solidity::CompilerStack const& _cs, std::v
 				{
 					//make sure there are no other contracts in the same source, otherwise it is not a rename
 					if (!std::any_of(result.begin(),result.end(), [=](ContractMap::const_iterator::value_type _v) { return _v != contract && _v->documentId() == contract->documentId(); }))
-					prevContract = c.value();
+						prevContract = c.value();
 				}
 		}
 		if (prevContract != nullptr && prevContract->contractInterface() != result[name]->contractInterface())
@@ -461,59 +484,59 @@ SolidityType CodeModel::nodeType(dev::solidity::Type const* _type)
 	switch (_type->getCategory())
 	{
 	case Type::Category::Integer:
-		{
+	{
-			IntegerType const* it = dynamic_cast<IntegerType const*>(_type);
+		IntegerType const* it = dynamic_cast<IntegerType const*>(_type);
-			r.size = it->getNumBits() / 8;
+		r.size = it->getNumBits() / 8;
-			r.type = it->isAddress() ? SolidityType::Type::Address : it->isSigned() ? SolidityType::Type::SignedInteger : SolidityType::Type::UnsignedInteger;
+		r.type = it->isAddress() ? SolidityType::Type::Address : it->isSigned() ? SolidityType::Type::SignedInteger : SolidityType::Type::UnsignedInteger;
-		}
+	}
 		break;
 	case Type::Category::Bool:
 		r.type = SolidityType::Type::Bool;
 		break;
 	case Type::Category::FixedBytes:
-		{
+	{
-			FixedBytesType const* b = dynamic_cast<FixedBytesType const*>(_type);
+		FixedBytesType const* b = dynamic_cast<FixedBytesType const*>(_type);
-			r.type = SolidityType::Type::Bytes;
+		r.type = SolidityType::Type::Bytes;
-			r.size = static_cast<unsigned>(b->getNumBytes());
+		r.size = static_cast<unsigned>(b->getNumBytes());
-		}
+	}
 		break;
 	case Type::Category::Contract:
 		r.type = SolidityType::Type::Address;
 		break;
 	case Type::Category::Array:
+	{
+		ArrayType const* array = dynamic_cast<ArrayType const*>(_type);
+		if (array->isByteArray())
+			r.type = SolidityType::Type::Bytes;
+		else
 		{
-			ArrayType const* array = dynamic_cast<ArrayType const*>(_type);
+			SolidityType elementType = nodeType(array->getBaseType().get());
-			if (array->isByteArray())
+			elementType.name = r.name;
-				r.type = SolidityType::Type::Bytes;
+			r = elementType;
-			else
-			{
-				SolidityType elementType = nodeType(array->getBaseType().get());
-				elementType.name = r.name;
-				r = elementType;
-			}
-			r.count = static_cast<unsigned>(array->getLength());
-			r.dynamicSize = _type->isDynamicallySized();
-			r.array = true;
 		}
+		r.count = static_cast<unsigned>(array->getLength());
+		r.dynamicSize = _type->isDynamicallySized();
+		r.array = true;
+	}
 		break;
 	case Type::Category::Enum:
-		{
+	{
-			r.type = SolidityType::Type::Enum;
+		r.type = SolidityType::Type::Enum;
-			EnumType const* e = dynamic_cast<EnumType const*>(_type);
+		EnumType const* e = dynamic_cast<EnumType const*>(_type);
-			for(auto const& enumValue: e->getEnumDefinition().getMembers())
+		for(auto const& enumValue: e->getEnumDefinition().getMembers())
-				r.enumNames.push_back(QString::fromStdString(enumValue->getName()));
+			r.enumNames.push_back(QString::fromStdString(enumValue->getName()));
-		}
+	}
 		break;
 	case Type::Category::Struct:
+	{
+		r.type = SolidityType::Type::Struct;
+		StructType const* s = dynamic_cast<StructType const*>(_type);
+		for(auto const& structMember: s->getMembers())
 		{
-			r.type = SolidityType::Type::Struct;
+			auto slotAndOffset = s->getStorageOffsetsOfMember(structMember.name);
-			StructType const* s = dynamic_cast<StructType const*>(_type);
+			r.members.push_back(SolidityDeclaration { QString::fromStdString(structMember.name), nodeType(structMember.type.get()), slotAndOffset.first, slotAndOffset.second });
-			for(auto const& structMember: s->getMembers())
-			{
-				auto slotAndOffset = s->getStorageOffsetsOfMember(structMember.name);
-				r.members.push_back(SolidityDeclaration { QString::fromStdString(structMember.name), nodeType(structMember.type.get()), slotAndOffset.first, slotAndOffset.second });
-			}
 		}
+	}
 		break;
 	case Type::Category::Function:
 	case Type::Category::IntegerConstant:

mix/CodeModel.h

@@ -216,7 +216,7 @@ signals:
 	/// Emitted on compilation complete
 	void compilationComplete();
 	/// Emitted on compilation error
-	void compilationError(QString _error, QString _sourceName);
+	void compilationError(QString _error, QVariantMap _firstErrorLoc, QVariantList _secondErrorLoc);
 	/// Internal signal used to transfer compilation job to background thread
 	void scheduleCompilationJob(int _jobId);
 	/// Emitted if there are any changes in the code model
@@ -239,6 +239,7 @@ private:
 	void stop();
 	void releaseContracts();
 	void collectContracts(dev::solidity::CompilerStack const& _cs, std::vector<std::string> const& _sourceNames);
+	QVariantMap resolveCompilationErrorLocation(dev::solidity::CompilerStack const& _cs, dev::SourceLocation const& _location);
 
 	std::atomic<bool> m_compiling;
 	mutable dev::Mutex x_contractMap;

mix/qml/CodeEditorView.qml

@@ -74,8 +74,8 @@ Item {
 			});
 		}
 		editor.document = document;
-		editor.sourceName = document.documentId;
 		editor.setFontSize(editorSettings.fontSize);
+		editor.sourceName = document.documentId;
 		editor.setText(data, document.syntaxMode);
 		editor.changeGeneration();
 	}
@@ -180,7 +180,7 @@ Item {
 	Connections {
 		target: codeModel
 		onCompilationError: {
-			sourceInError = _sourceName;
+			sourceInError = _firstErrorLoc.source;
 		}
 		onCompilationComplete: {
 			sourceInError = "";

mix/qml/StateListModel.qml

@@ -225,6 +225,7 @@ Item {
 				var ctorTr = defaultTransactionItem();
 				ctorTr.functionId = c;
 				ctorTr.contractId = c;
+				ctorTr.label = qsTr("Deploy") + " " + ctorTr.contractId;
 				ctorTr.sender = item.accounts[0].secret;
 				item.transactions.push(ctorTr);
 			}

mix/qml/WebCodeEditor.qml

@@ -152,17 +152,20 @@ Item {
 
 		}
 
-		function compilationError(error, sourceName)
+		function compilationError(error, firstLocation, secondLocations)
 		{
-			if (sourceName !== parent.sourceName)
-				return;
 			if (!editorBrowser || !error)
 				return;
-			var errorInfo = ErrorLocationFormater.extractErrorInfo(error, false);
+			var detail = error.split('\n')[0];
-			if (errorInfo.line && errorInfo.column)
+			var reg = detail.match(/:\d+:\d+:/g);
-				editorBrowser.runJavaScript("compilationError('" +  errorInfo.line + "', '" +  errorInfo.column + "', '" +  errorInfo.errorDetail + "')", function(result) { });
+			if (reg !== null)
-			else
+				detail = detail.replace(reg[0], "");
-				editorBrowser.runJavaScript("compilationComplete()", function(result) { });
+			displayErrorAnnotations(detail, firstLocation, secondLocations);
+		}
+
+		function displayErrorAnnotations(detail, location, secondaryErrors)
+		{
+			editorBrowser.runJavaScript("compilationError('" + sourceName + "', '" + JSON.stringify(location) + "', '" + detail + "', '" + JSON.stringify(secondaryErrors) + "')", function(result){});
 		}
 
 		Timer

mix/qml/html/cm/errorannotation.js

@@ -1,42 +1,39 @@
-function ErrorAnnotation(editor, line, column, content)
+function ErrorAnnotation(editor, location, content)
 {
+	this.location = location;
 	this.opened = false;
-	this.line = line;
+	this.rawContent = content;
-	this.column = column;
 	this.content = content.replace("Contract Error:", "");
 	this.editor = editor;
 	this.errorMark = null;
 	this.lineWidget = null;
 	this.init();
-	this.open();
+	if (this.content)
+		this.open();
 }
 
 ErrorAnnotation.prototype.init = function()
 {
-	var separators = [';', ',', '\\\(', '\\\{',  '\\\}', '\\\)', ':'];
+	this.errorMark = editor.markText({ line: this.location.start.line, ch: this.location.start.column }, { line: this.location.end.line, ch: this.location.end.column }, { className: "CodeMirror-errorannotation", inclusiveRight: true });
-	var errorPart = editor.getLine(this.line).substring(this.column);
-	var incrMark = this.column + errorPart.split(new RegExp(separators.join('|'), 'g'))[0].length;
-	if (incrMark === this.column)
-		incrMark = this.column + 1;
-	this.errorMark = editor.markText({ line: this.line, ch: this.column }, { line: this.line, ch: incrMark }, { className: "CodeMirror-errorannotation", inclusiveRight: true });
 }
 
 ErrorAnnotation.prototype.open = function()
 {
-	if (this.line)
+	if (this.location.start.line)
 	{
 		var node = document.createElement("div");
 		node.id = "annotation"
 		node.innerHTML = this.content;
 		node.className = "CodeMirror-errorannotation-context";
-		this.lineWidget = this.editor.addLineWidget(this.line, node, { coverGutter: false });
+		this.lineWidget = this.editor.addLineWidget(this.location.start.line, node, { coverGutter: false });
 		this.opened = true;
 	}
 }
 
 ErrorAnnotation.prototype.close = function()
 {
-	this.lineWidget.clear();
+	if (this.lineWidget)
+		this.lineWidget.clear();
 	this.opened = false;
 }
 

mix/qml/html/codeeditor.js

@@ -157,44 +157,49 @@ showWarning = function(content)
 	debugWarning = editor.addLineWidget(0, node, { coverGutter: false, above: true });
 }
 
-var annotation = null;
+var annotations = [];
 var compilationCompleteBool = true;
-compilationError = function(line, column, content)
+compilationError = function(currentSourceName, location, error, secondaryErrors)
 {
 	compilationCompleteBool = false;
-	window.setTimeout(function(){
+	if (compilationCompleteBool)
-		if (compilationCompleteBool)
+		return;
-			return;
+	clearAnnotations();
-		line = parseInt(line);
+	location = JSON.parse(location);
-		column = parseInt(column);
+	if (location.source === currentSourceName)
-		if (line > 0)
+		ensureAnnotation(location, error, "first");
-			line = line - 1;
+	var lineError = location.start.line + 1;
-		if (column > 0)
+	var errorOrigin = "Source " + location.contractName + " line " + lineError;
-			column = column - 1;
+	secondaryErrors = JSON.parse(secondaryErrors);
-
+	for(var i in secondaryErrors)
-		if (annotation == null)
+	{
-			annotation = new ErrorAnnotation(editor, line, column, content);
+		if (secondaryErrors[i].source === currentSourceName)
-		else if (annotation.line !== line || annotation.column !== column || annotation.content !== content)
+			ensureAnnotation(secondaryErrors[i], errorOrigin, "second");
-		{
+	}
-			annotation.destroy();
+}
-			annotation = new ErrorAnnotation(editor, line, column, content);
+
-		}
+ensureAnnotation = function(location, error, type)
-	}, 500)
+{
+	annotations.push({ "type": type, "annotation": new ErrorAnnotation(editor, location, error)});
+}
+
+clearAnnotations = function()
+{
+	for (var k in annotations)
+		annotations[k].annotation.destroy();
+	annotations.length = 0;
 }
 
 compilationComplete = function()
 {
-	if (annotation !== null)
+	clearAnnotations();
-	{
-		annotation.destroy();
-		annotation = null;
-	}
 	compilationCompleteBool = true;
 }
 
 goToCompilationError = function()
 {
-	editor.setCursor(annotation.line, annotation.column)
+	if (annotations.length > 0)
+		editor.setCursor(annotations[0].annotation.location.start.line, annotations[0].annotation.location.start.column)
 }
 
 setFontSize = function(size)