diff --git a/Arena.toml b/Arena.toml index adfb3bef..d48c5efc 100644 --- a/Arena.toml +++ b/Arena.toml @@ -12,7 +12,7 @@ commit_hash = "ee08634f28810e5d6fd1a904fc83f4e67821550e" [repositories."stjudecloud/workflows"] identifier = "stjudecloud/workflows" -commit_hash = "84bd23756563f32d144f11aa2e82a76e059a8b61" +commit_hash = "46a77b33c99c1952396fcaaf0845f1fb5a015987" filters = ["/template/task-templates.wdl"] [[diagnostics]] diff --git a/Gauntlet.toml b/Gauntlet.toml index 1ac93299..88f97398 100644 --- a/Gauntlet.toml +++ b/Gauntlet.toml @@ -8,7 +8,7 @@ commit_hash = "9a1a2846443e4b621f1dca0259fd3fa21885156a" [repositories."aws-samples/amazon-omics-tutorials"] identifier = "aws-samples/amazon-omics-tutorials" -commit_hash = "0a49b4326f67ac72150159a183084a252cf45ee0" +commit_hash = "136ecf7e5aaee18619e14ee65a97acc6d6704bef" [repositories."biowdl/tasks"] identifier = "biowdl/tasks" @@ -44,7 +44,7 @@ commit_hash = "ee08634f28810e5d6fd1a904fc83f4e67821550e" [repositories."stjudecloud/workflows"] identifier = "stjudecloud/workflows" -commit_hash = "84bd23756563f32d144f11aa2e82a76e059a8b61" +commit_hash = "46a77b33c99c1952396fcaaf0845f1fb5a015987" filters = ["/template/task-templates.wdl"] [repositories."theiagen/public_health_bioinformatics"] diff --git a/wdl-lint/CHANGELOG.md b/wdl-lint/CHANGELOG.md index ddcaec66..2589f2e0 100644 --- a/wdl-lint/CHANGELOG.md +++ b/wdl-lint/CHANGELOG.md @@ -9,6 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added +* Added the `PascalCase` lint rule ([#90](https://github.com/stjude-rust-labs/wdl/pull/90)). * Added the `ImportPlacement` lint rule ([#89](https://github.com/stjude-rust-labs/wdl/pull/89)). ### Fixed diff --git a/wdl-lint/RULES.md b/wdl-lint/RULES.md index 91fb9a0e..52ece3e7 100644 --- a/wdl-lint/RULES.md +++ b/wdl-lint/RULES.md @@ -15,7 +15,8 @@ be out of sync with released packages. | `MatchingParameterMeta` | Completeness | Ensures that inputs have a matching entry in a `parameter_meta` section. | | `MissingRuntime` | Completeness, Portability | Ensures that tasks have a runtime section. | | `NoCurlyCommands` | Clarity | Ensures that tasks use heredoc syntax in command sections. | +| `PascalCase` | Clarity, Naming, Style | Ensures that structs are defined with PascalCase names. | | `PreambleComments` | Clarity, Spacing, Style | Ensures that documents have correct comments in the preamble. | | `PreambleWhitespace` | Spacing, Style | Ensures that documents have correct whitespace in the preamble. | -| `SnakeCase` | Clarity, Naming, Style | Ensures that tasks, workflows, and variables use snake_case. | +| `SnakeCase` | Clarity, Naming, Style | Ensures that tasks, workflows, and variables are defined with snake_case names. | | `Whitespace` | Spacing, Style | Ensures that a document does not contain undesired whitespace. | diff --git a/wdl-lint/src/v1.rs b/wdl-lint/src/v1.rs index 892f8c8c..fec4e1f5 100644 --- a/wdl-lint/src/v1.rs +++ b/wdl-lint/src/v1.rs @@ -12,6 +12,7 @@ mod import_placement; mod matching_parameter_meta; mod missing_runtime; mod no_curly_commands; +mod pascal_case; mod preamble_comments; mod preamble_whitespace; mod snake_case; @@ -24,6 +25,7 @@ pub use import_placement::*; pub use matching_parameter_meta::*; pub use missing_runtime::*; pub use no_curly_commands::*; +pub use pascal_case::*; pub use preamble_comments::*; pub use preamble_whitespace::*; pub use snake_case::*; @@ -71,6 +73,7 @@ pub fn rules() -> Vec> { Box::new(WhitespaceRule), Box::new(CommandSectionMixedIndentationRule), Box::new(ImportPlacementRule), + Box::new(PascalCaseRule), ]; // Ensure all the rule ids are unique and pascal case diff --git a/wdl-lint/src/v1/pascal_case.rs b/wdl-lint/src/v1/pascal_case.rs new file mode 100644 index 00000000..17e7b445 --- /dev/null +++ b/wdl-lint/src/v1/pascal_case.rs @@ -0,0 +1,87 @@ +//! A lint rule that ensures structs are defined with pascal case names. + +use convert_case::Boundary; +use convert_case::Case; +use convert_case::Converter; +use wdl_ast::v1::StructDefinition; +use wdl_ast::v1::Visitor; +use wdl_ast::AstToken; +use wdl_ast::Diagnostic; +use wdl_ast::Diagnostics; +use wdl_ast::Span; +use wdl_ast::VisitReason; + +use super::Rule; +use crate::Tag; +use crate::TagSet; + +/// The identifier for the pascal case rule. +const ID: &str = "PascalCase"; + +/// Creates a "use pascal case" diagnostic. +fn use_pascal_case(name: &str, properly_cased_name: &str, span: Span) -> Diagnostic { + Diagnostic::warning(format!("struct name `{name}` is not PascalCase")) + .with_rule(ID) + .with_label("this name must be PascalCase", span) + .with_fix(format!("replace `{name}` with `{properly_cased_name}`")) +} + +/// Detects structs defined without a pascal case name. +#[derive(Debug, Clone, Copy)] +pub struct PascalCaseRule; + +impl Rule for PascalCaseRule { + fn id(&self) -> &'static str { + ID + } + + fn description(&self) -> &'static str { + "Ensures that structs are defined with PascalCase names." + } + + fn explanation(&self) -> &'static str { + "Struct names should be in PascalCase. Maintaining a consistent naming convention makes \ + the code easier to read and understand." + } + + fn tags(&self) -> TagSet { + TagSet::new(&[Tag::Naming, Tag::Style, Tag::Clarity]) + } + + fn visitor(&self) -> Box> { + Box::new(PascalCaseVisitor) + } +} + +/// Checks if the given name is pascal case, and if not adds a warning to the +/// diagnostics. +fn check_name(name: &str, span: Span, diagnostics: &mut Diagnostics) { + let converter = Converter::new() + .remove_boundaries(&[Boundary::DigitLower, Boundary::LowerDigit]) + .to_case(Case::Pascal); + let properly_cased_name = converter.convert(name); + if name != properly_cased_name { + diagnostics.add(use_pascal_case(name, &properly_cased_name, span)); + } +} + +/// Implements the visitor for the pascal case rule. +struct PascalCaseVisitor; + +impl Visitor for PascalCaseVisitor { + type State = Diagnostics; + + fn struct_definition( + &mut self, + state: &mut Self::State, + reason: VisitReason, + def: &StructDefinition, + ) { + if reason == VisitReason::Exit { + return; + } + + let name = def.name(); + check_name(name.as_str(), name.span(), state); + } +} diff --git a/wdl-lint/src/v1/snake_case.rs b/wdl-lint/src/v1/snake_case.rs index f2185ccb..f1044110 100644 --- a/wdl-lint/src/v1/snake_case.rs +++ b/wdl-lint/src/v1/snake_case.rs @@ -88,7 +88,7 @@ impl Rule for SnakeCaseRule { } fn description(&self) -> &'static str { - "Ensures that tasks, workflows, and variables use snake_case." + "Ensures that tasks, workflows, and variables are defined with snake_case names." } fn explanation(&self) -> &'static str { diff --git a/wdl-lint/tests/lints/pascal-case/source.errors b/wdl-lint/tests/lints/pascal-case/source.errors new file mode 100644 index 00000000..6df215f9 --- /dev/null +++ b/wdl-lint/tests/lints/pascal-case/source.errors @@ -0,0 +1,24 @@ +warning[PascalCase]: struct name `this_is_a_bad_name` is not PascalCase + ┌─ tests/lints/pascal-case/source.wdl:5:8 + │ +5 │ struct this_is_a_bad_name { + │ ^^^^^^^^^^^^^^^^^^ this name must be PascalCase + │ + = fix: replace `this_is_a_bad_name` with `ThisIsABadName` + +warning[PascalCase]: struct name `thisIsAlsoABadName` is not PascalCase + ┌─ tests/lints/pascal-case/source.wdl:9:8 + │ +9 │ struct thisIsAlsoABadName { + │ ^^^^^^^^^^^^^^^^^^ this name must be PascalCase + │ + = fix: replace `thisIsAlsoABadName` with `ThisIsAlsoABadName` + +warning[PascalCase]: struct name `This_Is_Bad_Too` is not PascalCase + ┌─ tests/lints/pascal-case/source.wdl:13:8 + │ +13 │ struct This_Is_Bad_Too { + │ ^^^^^^^^^^^^^^^ this name must be PascalCase + │ + = fix: replace `This_Is_Bad_Too` with `ThisIsBadToo` + diff --git a/wdl-lint/tests/lints/pascal-case/source.wdl b/wdl-lint/tests/lints/pascal-case/source.wdl new file mode 100644 index 00000000..c9627ff0 --- /dev/null +++ b/wdl-lint/tests/lints/pascal-case/source.wdl @@ -0,0 +1,19 @@ +## This is a test of ensuring struct names are PascalCase. + +version 1.1 + +struct this_is_a_bad_name { + Int x +} + +struct thisIsAlsoABadName { + Int x +} + +struct This_Is_Bad_Too { + Int x +} + +struct ThisNameIsAGoodOne { + Int x +}