Skip to content

Improve error message for invalid target in attribute #4369

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Mar 19, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,10 @@
and patterns.
([Richard Viney](https://github.com/richard-viney))

- Improve the error messages for unknown and missing target names
in target attributes.
([Alexander Keleschovsky](https://github.com/AlecGhost))

### Build tool

- Include a type annotation for the `main` function generated by `gleam new`.
Expand Down
22 changes: 11 additions & 11 deletions compiler-core/src/parse.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3482,8 +3482,10 @@ functions are declared separately from types.";
}
}

// Expect a target name. e.g. `javascript` or `erlang`
fn expect_target(&mut self) -> Result<Target, ParseError> {
// Expect a target name. e.g. `javascript` or `erlang`.
// The location of the preceding left parenthesis is required
// to give the correct error span in case the target name is missing.
fn expect_target(&mut self, paren_location: SrcSpan) -> Result<Target, ParseError> {
let (start, t, end) = match self.next_tok() {
Some(t) => t,
None => {
Expand All @@ -3497,22 +3499,22 @@ functions are declared separately from types.";
"js" => {
self.warnings
.push(DeprecatedSyntaxWarning::DeprecatedTargetShorthand {
location: SrcSpan { start, end },
location: SrcSpan::new(start, end),
target: Target::JavaScript,
});
Ok(Target::JavaScript)
}
"erl" => {
self.warnings
.push(DeprecatedSyntaxWarning::DeprecatedTargetShorthand {
location: SrcSpan { start, end },
location: SrcSpan::new(start, end),
target: Target::Erlang,
});
Ok(Target::Erlang)
}
_ => self.next_tok_unexpected(Target::variant_strings()),
_ => parse_error(ParseErrorType::UnknownTarget, SrcSpan::new(start, end)),
},
_ => self.next_tok_unexpected(Target::variant_strings()),
_ => parse_error(ParseErrorType::ExpectedTargetName, paren_location),
}
}

Expand Down Expand Up @@ -3761,10 +3763,7 @@ functions are declared separately from types.";
let _ = self.expect_one(&Token::LeftParen)?;
self.parse_external_attribute(start, end, attributes)
}
"target" => {
let _ = self.expect_one(&Token::LeftParen)?;
self.parse_target_attribute(start, end, attributes)
}
"target" => self.parse_target_attribute(start, end, attributes),
"deprecated" => {
let _ = self.expect_one(&Token::LeftParen)?;
self.parse_deprecated_attribute(start, end, attributes)
Expand All @@ -3782,7 +3781,8 @@ functions are declared separately from types.";
end: u32,
attributes: &mut Attributes,
) -> Result<u32, ParseError> {
let target = self.expect_target()?;
let (paren_start, paren_end) = self.expect_one(&Token::LeftParen)?;
let target = self.expect_target(SrcSpan::new(paren_start, paren_end))?;
if attributes.target.is_some() {
return parse_error(ParseErrorType::DuplicateAttribute, SrcSpan { start, end });
}
Expand Down
5 changes: 5 additions & 0 deletions compiler-core/src/parse/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,10 @@ impl ParseError {
ParseErrorType::ExpectedFunctionDefinition => {
("I was expecting a function definition after this", vec![])
}
ParseErrorType::ExpectedTargetName => (
"I was expecting a target name after this",
vec!["Try `erlang`, `javascript`.".into()],
),
ParseErrorType::ExtraSeparator => (
"This is an extra delimiter",
vec!["Hint: Try removing it?".into()],
Expand Down Expand Up @@ -351,6 +355,7 @@ pub enum ParseErrorType {
ExpectedDefinition, // after attributes
ExpectedDeprecationMessage, // after "deprecated"
ExpectedFunctionDefinition, // after function-only attributes
ExpectedTargetName, // after "@target("
ExprLparStart, // it seems "(" was used to start an expression
ExtraSeparator, // #(1,,) <- the 2nd comma is an extra separator
IncorrectName, // UpName or DiscardName used when Name was expected
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
---
source: compiler-core/src/parse/tests.rs
expression: "\n@target()\npub fn one() {}"
---
----- SOURCE CODE

@target()
pub fn one() {}

----- ERROR
error: Syntax error
┌─ /src/parse/error.gleam:2:8
2 │ @target()
│ ^ I was expecting a target name after this

Try `erlang`, `javascript`.
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
---
source: compiler-core/src/parse/tests.rs
expression: "\n@target(\npub fn one() {}"
---
----- SOURCE CODE

@target(
pub fn one() {}

----- ERROR
error: Syntax error
┌─ /src/parse/error.gleam:2:8
2 │ @target(
│ ^ I was expecting a target name after this

Try `erlang`, `javascript`.
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
---
source: compiler-core/src/parse/tests.rs
expression: "\n@target(abc)\npub fn one() {}"
---
----- SOURCE CODE

@target(abc)
pub fn one() {}

----- ERROR
error: Syntax error
┌─ /src/parse/error.gleam:2:9
2 │ @target(abc)
│ ^^^ I don't recognise this target

Try `erlang`, `javascript`.
27 changes: 27 additions & 0 deletions compiler-core/src/parse/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -659,6 +659,33 @@ pub fn one(x: Int) -> Int {
);
}

#[test]
fn unknown_target() {
assert_module_error!(
r#"
@target(abc)
pub fn one() {}"#
);
}

#[test]
fn missing_target() {
assert_module_error!(
r#"
@target()
pub fn one() {}"#
);
}

#[test]
fn missing_target_and_bracket() {
assert_module_error!(
r#"
@target(
pub fn one() {}"#
);
}

#[test]
fn unknown_attribute() {
assert_module_error!(
Expand Down
Loading