diff --git a/slicec/src/validators/comments.rs b/slicec/src/validators/comments.rs index 1e5db6b2..2eb98bbb 100644 --- a/slicec/src/validators/comments.rs +++ b/slicec/src/validators/comments.rs @@ -8,29 +8,42 @@ pub fn validate_common_doc_comments(commentable: &dyn Commentable, diagnostics: // Only run this validation if a doc comment is present. let Some(comment) = commentable.comment() else { return }; - only_operations_have_parameters(comment, commentable, diagnostics); - only_operations_can_return(comment, commentable, diagnostics); + validate_param_tag_placement(comment, commentable, diagnostics); + validate_returns_tag_placement(comment, commentable, diagnostics); } -fn only_operations_have_parameters(comment: &DocComment, entity: &dyn Commentable, diagnostics: &mut Diagnostics) { +fn validate_param_tag_placement(comment: &DocComment, entity: &dyn Commentable, diagnostics: &mut Diagnostics) { let concrete_entity = entity.concrete_entity(); if !matches!(concrete_entity, Entities::Operation(_) | Entities::Enumerator(_)) { for param_tag in &comment.params { - report_only_operation_error(param_tag, param_tag.message.span(), entity, diagnostics); + report_only_operation_error( + "comment has a 'param' tag, but only operations and enumerators have parameters".to_owned(), + param_tag, + param_tag.message.span(), + entity, + diagnostics, + ); } } } -fn only_operations_can_return(comment: &DocComment, entity: &dyn Commentable, diagnostics: &mut Diagnostics) { +fn validate_returns_tag_placement(comment: &DocComment, entity: &dyn Commentable, diagnostics: &mut Diagnostics) { if !matches!(entity.concrete_entity(), Entities::Operation(_)) { for returns_tag in &comment.returns { - report_only_operation_error(returns_tag, returns_tag.message.span(), entity, diagnostics); + report_only_operation_error( + "comment has a 'returns' tag, but only operations have return types".to_owned(), + returns_tag, + returns_tag.message.span(), + entity, + diagnostics, + ); } } } /// Helper function that reports an error if an operation-only comment-tag was used on something other than a comment. fn report_only_operation_error( + message: String, tag: &impl Symbol, message_span: &Span, entity: &dyn Commentable, @@ -43,19 +56,9 @@ fn report_only_operation_error( a = crate::utils::string_util::indefinite_article(entity_kind), ); - // All tag kinds are of the form " tag", so it's safe to unwrap. We only want the first word for the message. - let tag_kind = tag.kind().split_once(' ').unwrap().0; - let action_phrase = match tag_kind { - "param" => "have parameters", - "returns" => "return", - _ => unreachable!("'report_only_operation_error' was called with unsupported tag '{tag_kind}'"), - }; - - Diagnostic::from_lint(Lint::IncorrectDocComment { - message: format!("comment has a '{tag_kind}' tag, but only operations can {action_phrase}"), - }) - .set_span(&(tag.span() + message_span)) - .set_scope(entity.parser_scoped_identifier()) - .add_note(note, Some(entity.span())) - .push_into(diagnostics); + Diagnostic::from_lint(Lint::IncorrectDocComment { message }) + .set_span(&(tag.span() + message_span)) + .set_scope(entity.parser_scoped_identifier()) + .add_note(note, Some(entity.span())) + .push_into(diagnostics); } diff --git a/slicec/src/validators/enums.rs b/slicec/src/validators/enums.rs index ca55a45c..cb52276f 100644 --- a/slicec/src/validators/enums.rs +++ b/slicec/src/validators/enums.rs @@ -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; @@ -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. @@ -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); + } + } +} diff --git a/slicec/tests/attribute_tests.rs b/slicec/tests/attribute_tests.rs index e7d720a8..1a22dd41 100644 --- a/slicec/tests/attribute_tests.rs +++ b/slicec/tests/attribute_tests.rs @@ -123,7 +123,7 @@ mod attributes { message: "no element named 'fake' exists in scope".to_owned(), }), Diagnostic::from_lint(Lint::IncorrectDocComment { - message: "comment has a 'returns' tag, but only operations can return".to_owned(), + message: "comment has a 'returns' tag, but only operations have return types".to_owned(), }), ]; for (index, lint) in updated_diagnostics.iter().enumerate() { diff --git a/slicec/tests/comment_tests.rs b/slicec/tests/comment_tests.rs index e4d56424..5fd1355d 100644 --- a/slicec/tests/comment_tests.rs +++ b/slicec/tests/comment_tests.rs @@ -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 @@ -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_field() { + // 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 @@ -513,7 +574,7 @@ mod comments { } #[test] - fn param_tags_can_only_be_used_with_operations() { + fn param_tags_are_rejected_on_incorrect_elements() { // Arrange let slice = " module tests @@ -527,13 +588,13 @@ mod comments { // Assert let expected = Diagnostic::from_lint(Lint::IncorrectDocComment { - message: "comment has a 'param' tag, but only operations can have parameters".to_owned(), + message: "comment has a 'param' tag, but only operations and enumerators have parameters".to_owned(), }); check_diagnostics(diagnostics, [expected]); } #[test] - fn returns_tags_can_only_be_used_with_operations() { + fn returns_tags_are_rejected_on_incorrect_elements() { // Arrange let slice = " module tests @@ -547,7 +608,7 @@ mod comments { // Assert let expected = Diagnostic::from_lint(Lint::IncorrectDocComment { - message: "comment has a 'returns' tag, but only operations can return".to_owned(), + message: "comment has a 'returns' tag, but only operations have return types".to_owned(), }); check_diagnostics(diagnostics, [expected]); }