Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
27 changes: 26 additions & 1 deletion slicec/src/validators/enums.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
// Copyright (c) ZeroC, Inc.

use crate::diagnostics::{Diagnostic, Diagnostics, Error};
use crate::diagnostics::{Diagnostic, Diagnostics, Error, Lint};
use crate::grammar::*;

use std::collections::HashMap;
Expand All @@ -17,6 +17,10 @@ pub fn validate_enum(enum_def: &Enum, diagnostics: &mut Diagnostics) {
if enum_def.underlying.is_some() {
cannot_contain_fields(enum_def, diagnostics);
}

for enumerator in enum_def.enumerators() {
validate_param_tags(enumerator, diagnostics);
}
}

/// Validate that the enumerators are within the bounds of the specified underlying type.
Expand Down Expand Up @@ -182,3 +186,24 @@ fn compact_enums_cannot_contain_tags(enum_def: &Enum, diagnostics: &mut Diagnost
}
}
}

/// Validates that any `@param` tags on an enumerator are valid (i.e. they refer to actual fields of the enumerator).
fn validate_param_tags(enumerator: &Enumerator, diagnostics: &mut Diagnostics) {
let Some(comment) = enumerator.comment() else { return };

let fields: Vec<_> = enumerator.fields().iter().map(|f| f.identifier()).collect();
for param_tag in &comment.params {
let tag_identifier = param_tag.identifier.value.as_str();
if !fields.contains(&tag_identifier) {
Diagnostic::from_lint(Lint::IncorrectDocComment {
message: format!(
"comment has a 'param' tag for '{tag_identifier}', but enumerator '{}' has no field with that name",
enumerator.identifier(),
),
})
.set_span(param_tag.span())
.set_scope(enumerator.parser_scoped_identifier())
.push_into(diagnostics);
}
}
}
65 changes: 63 additions & 2 deletions slicec/tests/comment_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -208,6 +208,23 @@ mod comments {
check_diagnostics(diagnostics, expected);
}

#[test]
fn enumerator_with_correct_doc_comments() {
// Arrange
let slice = "
module tests

enum E {
/// This enumerator has 2 fields.
/// @param testParam1: A string param
A(testParam1: string, testParam2: bool)
}
";

// Act/Assert
assert_parses(slice);
}

#[test]
fn operation_with_correct_doc_comments() {
// Arrange
Expand Down Expand Up @@ -395,6 +412,50 @@ mod comments {
check_diagnostics(diagnostics, [expected]);
}

#[test]
fn param_tag_is_rejected_for_enumerators_with_no_fields() {
// Arrange
let slice = "
module tests

enum E {
/// @param foo: this parameter doesn't exist.
A
}
";

// Act
let diagnostics = parse_for_diagnostics(slice);

// Assert
let expected = Diagnostic::from_lint(Lint::IncorrectDocComment {
message: "comment has a 'param' tag for 'foo', but enumerator 'A' has no field with that name".to_owned(),
});
check_diagnostics(diagnostics, [expected]);
}

#[test]
fn param_tag_is_rejected_if_its_identifier_does_not_match_a_fields() {
Comment thread
InsertCreativityHere marked this conversation as resolved.
Outdated
// Arrange
let slice = "
module tests

enum E {
/// @param foo: this parameter doesn't exist.
A(bar: bool)
}
";

// Act
let diagnostics = parse_for_diagnostics(slice);

// Assert
let expected = Diagnostic::from_lint(Lint::IncorrectDocComment {
message: "comment has a 'param' tag for 'foo', but enumerator 'A' has no field with that name".to_owned(),
});
check_diagnostics(diagnostics, [expected]);
}

#[test]
fn param_tag_is_rejected_for_operations_with_no_parameters() {
// Arrange
Expand Down Expand Up @@ -513,7 +574,7 @@ mod comments {
}

#[test]
fn param_tags_can_only_be_used_with_operations() {
fn param_tags_are_rejected_on_incorrect_elements() {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we update this expected message and the corresponding diagnostic in validators/comments.rs? Enumerators are now valid owners of @PARAM tags, so 'only operations can have parameters' is misleading. For example: 'comment has a param tag, but only operations and enumerators can use param tags'.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated the message, and the names of the validation functions around it!

// Arrange
let slice = "
module tests
Expand All @@ -533,7 +594,7 @@ mod comments {
}

#[test]
fn returns_tags_can_only_be_used_with_operations() {
fn returns_tags_are_rejected_on_incorrect_elements() {
// Arrange
let slice = "
module tests
Expand Down