Error reporting of use of non-symbols in indentifier context

src/errors.cc

@@ -119,7 +119,8 @@ std::ostream& operator<<(std::ostream& os, Error const& err)
 		[](errors::Unexpected_Keyword const&)                   { return "Unexpected keyword"; },
 		[](errors::Unrecognized_Character const&)               { return "Unrecognized character"; },
 		[](errors::internal::Unexpected_Token const&)           { return "Unexpected token"; },
-		[](errors::Expected_Expression_Separator_Before const&) { return "Missing semicolon"; }
+		[](errors::Expected_Expression_Separator_Before const&) { return "Missing semicolon"; },
+		[](errors::Literal_As_Identifier const&)                { return "Literal used in place of an identifier"; }
 	}, err.details);
 
 	error_heading(os, err.location, Error_Level::Error, short_description);
@@ -161,6 +162,20 @@ std::ostream& operator<<(std::ostream& os, Error const& err)
 			}
 		},
 
+		[&os](errors::Literal_As_Identifier const& err) {
+			os << "I expected an identifier in " << err.context << ", but found" << (err.type_name.empty() ? "" : " ") << err.type_name << " value = '" << err.source << "'\n";
+
+
+			if (err.type_name == "chord") {
+				os << "\nTry renaming to different name or appending with something that is not part of chord literal like 'x'\n";
+
+				os << pretty::begin_comment <<
+					"\nMusical notation names are reserved for chord and note notations,\n"
+					"and cannot be reused as an identifier to prevent ambiguity\n"
+					<< pretty::end;
+			}
+		},
+
 		[&os](errors::Not_Callable const&)                     { unimplemented(); },
 		[&os](errors::Undefined_Operator const&)               { unimplemented(); },
 		[&os](errors::Unexpected_Keyword const&)               { unimplemented(); },

src/musique.hh

@@ -83,6 +83,14 @@ namespace errors
 		std::string_view type;
 	};
 
+	// When user provides literal where identifier should be
+	struct Literal_As_Identifier
+	{
+		std::string_view type_name;
+		std::string_view source;
+		std::string_view context;
+	};
+
 	/// Collection of messages that are considered internal and should not be printed to the end user.
 	namespace internal
 	{
@@ -109,6 +117,7 @@ namespace errors
 		Unexpected_Empty_Source,
 		Unexpected_Keyword,
 		Unrecognized_Character,
+		Literal_As_Identifier,
 		internal::Unexpected_Token
 	>;
 }

src/parser.cc

@@ -3,6 +3,18 @@
 
 static Ast wrap_if_several(std::vector<Ast> &&ast, Ast(*wrapper)(std::vector<Ast>));
 
+static inline bool is_identifier_looking(Token::Type type)
+{
+	switch (type) {
+	case Token::Type::Symbol:
+	case Token::Type::Keyword:
+	case Token::Type::Chord:
+		return true;
+	default:
+		return false;
+	}
+}
+
 constexpr auto Literal_Keywords = std::array {
 	"false"sv,
 	"nil"sv,
@@ -39,7 +51,7 @@ Result<Ast> Parser::parse(std::string_view source, std::string_view filename)
 
 	auto const result = parser.parse_sequence();
 
-	if (parser.token_id < parser.tokens.size()) {
+	if (result.has_value() && parser.token_id < parser.tokens.size()) {
 		if (parser.expect(Token::Type::Keyword, "var")) {
 			return Error {
 				.details = errors::Expected_Expression_Separator_Before { .what = "var" },
@@ -71,14 +83,29 @@ Result<Ast> Parser::parse_variable_declaration()
 	assert(expect(Token::Type::Keyword, "var"), "Parser::parse_variable_declaration must be called only on expressions that starts with 'var'");
 	auto var = consume();
 
-	auto lvalue = Try(parse_many(*this, &Parser::parse_identifier, std::nullopt, At_Least::One));
+	auto lvalue = parse_many(*this, &Parser::parse_identifier, std::nullopt, At_Least::One);
+	if (not lvalue.has_value()) {
+		auto details = lvalue.error().details;
+		if (auto ut = std::get_if<errors::internal::Unexpected_Token>(&details); ut) {
+			return Error {
+				.details = errors::Literal_As_Identifier {
+					.type_name = ut->type,
+					.source = ut->source,
+					.context = "variable declaration"
+				},
+				.location = lvalue.error().location
+			};
+		}
+		return std::move(lvalue).error();
+	}
+
 
 	if (expect(Token::Type::Operator, "=")) {
 		consume();
-		return Ast::variable_declaration(var.location, std::move(lvalue), Try(parse_expression()));
+		return Ast::variable_declaration(var.location, *std::move(lvalue), Try(parse_expression()));
 	}
 
-	return Ast::variable_declaration(var.location, std::move(lvalue), std::nullopt);
+	return Ast::variable_declaration(var.location, *std::move(lvalue), std::nullopt);
 }
 
 Result<Ast> Parser::parse_infix_expression()
@@ -143,7 +170,35 @@ Result<Ast> Parser::parse_atomic_expression()
 
 			return parse_sequence().and_then([&](Ast &&ast) -> Result<Ast> {
 				if (not expect(Token::Type::Close_Block)) {
-					unimplemented("Error handling of this code is not implemented yet");
+					if (expect(Token::Type::Parameter_Separator)) {
+						if (is_lambda) {
+							assert(false, "There should be error message that you cannot put multiple parameter separators in one block");
+						} else {
+							// This may be a result of user trying to specify parameters from things that cannot be parameters (like chord literals)
+							// or accidential hit of "|" on the keyboard. We can detect first case by ensuring that all tokens between this place
+							// and beggining of a block are identifier looking: keywords, symbols, literal chord declarations, boolean literals etc
+							std::optional<Token> invalid_token = std::nullopt;
+							auto const success = std::all_of(tokens.begin() + start, tokens.begin() + (token_id - start + 1), [&](Token const& token) {
+								if (!invalid_token && token.type != Token::Type::Symbol) {
+									// TODO Maybe gather all tokens to provide all the help needed in the one iteration
+									invalid_token = token;
+								}
+								return is_identifier_looking(token.type);
+							});
+
+							if (success && invalid_token) {
+								return Error {
+									.details = errors::Literal_As_Identifier {
+										.type_name = type_name(invalid_token->type),
+										.source = invalid_token->source,
+										.context = "block parameter list"
+									},
+									.location = invalid_token->location
+								};
+							}
+						}
+					}
+					unimplemented("Don't know why it stopped, maybe end of file?");
 				}
 				consume();
 				if (is_lambda) {
@@ -182,7 +237,7 @@ Result<Ast> Parser::parse_identifier_with_trailing_separators()
 		// TODO Specific error message
 		return Error {
 			.details = errors::internal::Unexpected_Token {
-				.type = "", // TODO fill type
+				.type = type_name(peek()->type),
 				.source = peek()->source,
 				.when = "identifier parsing"
 			},
@@ -200,7 +255,7 @@ Result<Ast> Parser::parse_identifier()
 		// TODO Specific error message
 		return Error {
 			.details = errors::internal::Unexpected_Token {
-				.type = "", // TODO fill type
+				.type = type_name(peek()->type),
 				.source = peek()->source,
 				.when = "identifier parsing"
 			},