feat: InputNotSorted lint rule (#100)

Co-authored-by: Andrew Frantz <andrew.frantz@stjude.org>

Gauntlet.toml

@@ -44,7 +44,7 @@ commit_hash = "ee08634f28810e5d6fd1a904fc83f4e67821550e"
 
 [repositories."stjudecloud/workflows"]
 identifier = "stjudecloud/workflows"
-commit_hash = "46a77b33c99c1952396fcaaf0845f1fb5a015987"
+commit_hash = "f67825369c2459e4b5a0ae970873e462af33d7b0"
 filters = ["/template/task-templates.wdl"]
 
 [repositories."theiagen/public_health_bioinformatics"]

wdl-ast/src/v1/decls.rs

@@ -74,6 +74,18 @@ impl fmt::Display for MapType {
     }
 }
 
+impl PartialOrd for MapType {
+    fn partial_cmp(&self, other: &Self) -> Option<std::cmp::Ordering> {
+        Some(self.cmp(other))
+    }
+}
+
+impl Ord for MapType {
+    fn cmp(&self, other: &Self) -> std::cmp::Ordering {
+        self.types().cmp(&other.types())
+    }
+}
+
 /// Represents an `Array` type.
 #[derive(Clone, Debug, PartialEq, Eq)]
 pub struct ArrayType(SyntaxNode);
@@ -135,6 +147,24 @@ impl fmt::Display for ArrayType {
     }
 }
 
+impl PartialOrd for ArrayType {
+    fn partial_cmp(&self, other: &Self) -> Option<std::cmp::Ordering> {
+        Some(self.cmp(other))
+    }
+}
+
+impl Ord for ArrayType {
+    fn cmp(&self, other: &Self) -> std::cmp::Ordering {
+        if self.is_non_empty() && !other.is_non_empty() {
+            std::cmp::Ordering::Less
+        } else if !self.is_non_empty() && other.is_non_empty() {
+            std::cmp::Ordering::Greater
+        } else {
+            self.element_type().cmp(&other.element_type())
+        }
+    }
+}
+
 /// Represents a `Pair` type.
 #[derive(Clone, Debug, PartialEq, Eq)]
 pub struct PairType(SyntaxNode);
@@ -193,6 +223,18 @@ impl fmt::Display for PairType {
     }
 }
 
+impl PartialOrd for PairType {
+    fn partial_cmp(&self, other: &Self) -> Option<std::cmp::Ordering> {
+        Some(self.cmp(other))
+    }
+}
+
+impl Ord for PairType {
+    fn cmp(&self, other: &Self) -> std::cmp::Ordering {
+        self.types().cmp(&other.types())
+    }
+}
+
 /// Represents a `Object` type.
 #[derive(Clone, Debug, PartialEq, Eq)]
 pub struct ObjectType(SyntaxNode);
@@ -242,6 +284,18 @@ impl fmt::Display for ObjectType {
     }
 }
 
+impl PartialOrd for ObjectType {
+    fn partial_cmp(&self, other: &Self) -> Option<std::cmp::Ordering> {
+        Some(self.cmp(other))
+    }
+}
+
+impl Ord for ObjectType {
+    fn cmp(&self, _: &Self) -> std::cmp::Ordering {
+        std::cmp::Ordering::Equal
+    }
+}
+
 /// Represents a reference to a type.
 #[derive(Clone, Debug, PartialEq, Eq)]
 pub struct TypeRef(SyntaxNode);
@@ -297,6 +351,18 @@ impl fmt::Display for TypeRef {
     }
 }
 
+impl PartialOrd for TypeRef {
+    fn partial_cmp(&self, other: &Self) -> Option<std::cmp::Ordering> {
+        Some(self.cmp(other))
+    }
+}
+
+impl Ord for TypeRef {
+    fn cmp(&self, _: &Self) -> std::cmp::Ordering {
+        std::cmp::Ordering::Equal
+    }
+}
+
 /// Represents a kind of primitive type.
 #[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash)]
 pub enum PrimitiveTypeKind {
@@ -339,6 +405,17 @@ impl PrimitiveType {
             Some(SyntaxKind::QuestionMark)
         )
     }
+
+    /// Defines an ordering for PrimitiveTypes
+    fn primitive_type_index(&self) -> usize {
+        match self.kind() {
+            PrimitiveTypeKind::Boolean => 2,
+            PrimitiveTypeKind::Integer => 4,
+            PrimitiveTypeKind::Float => 3,
+            PrimitiveTypeKind::String => 1,
+            PrimitiveTypeKind::File => 0,
+        }
+    }
 }
 
 impl AstNode for PrimitiveType {
@@ -384,6 +461,19 @@ impl fmt::Display for PrimitiveType {
     }
 }
 
+impl PartialOrd for PrimitiveType {
+    fn partial_cmp(&self, other: &Self) -> Option<std::cmp::Ordering> {
+        Some(self.cmp(other))
+    }
+}
+
+impl Ord for PrimitiveType {
+    fn cmp(&self, other: &Self) -> std::cmp::Ordering {
+        self.primitive_type_index()
+            .cmp(&other.primitive_type_index())
+    }
+}
+
 /// Represents a type.
 #[derive(Clone, Debug, PartialEq, Eq)]
 pub enum Type {
@@ -455,7 +545,7 @@ impl Type {
     /// # Panics
     ///
     /// Panics if the type is not an object type.
-    pub fn unwrap_objet_type(self) -> ObjectType {
+    pub fn unwrap_object_type(self) -> ObjectType {
         match self {
             Self::Object(ty) => ty,
             _ => panic!("not an object type"),
@@ -485,6 +575,27 @@ impl Type {
             _ => panic!("not a primitive type"),
         }
     }
+
+    /// Defines an ordering for types.
+    fn type_index(&self) -> usize {
+        match self {
+            Type::Map(_) => 5,
+            Type::Array(a) => match a.is_non_empty() {
+                true => 1,
+                false => 2,
+            },
+            Type::Pair(_) => 6,
+            Type::Object(_) => 4,
+            Type::Ref(_) => 3,
+            Type::Primitive(p) => match p.kind() {
+                PrimitiveTypeKind::Boolean => 8,
+                PrimitiveTypeKind::Integer => 10,
+                PrimitiveTypeKind::Float => 9,
+                PrimitiveTypeKind::String => 7,
+                PrimitiveTypeKind::File => 0,
+            },
+        }
+    }
 }
 
 impl AstNode for Type {
@@ -545,6 +656,31 @@ impl fmt::Display for Type {
     }
 }
 
+impl PartialOrd for Type {
+    fn partial_cmp(&self, other: &Self) -> Option<std::cmp::Ordering> {
+        Some(self.cmp(other))
+    }
+}
+
+impl Ord for Type {
+    fn cmp(&self, other: &Self) -> std::cmp::Ordering {
+        compare_types(self, other)
+    }
+}
+
+/// Compare all variants of Type.
+fn compare_types(a: &Type, b: &Type) -> std::cmp::Ordering {
+    // Check Array, Map, and Pair for sub-types
+    match (a, b) {
+        (Type::Map(a), Type::Map(b)) => a.cmp(b),
+        (Type::Array(a), Type::Array(b)) => a.cmp(b),
+        (Type::Pair(a), Type::Pair(b)) => a.cmp(b),
+        (Type::Ref(a), Type::Ref(b)) => a.cmp(b),
+        (Type::Object(a), Type::Object(b)) => a.cmp(b),
+        _ => a.type_index().cmp(&b.type_index()),
+    }
+}
+
 /// Represents an unbound declaration.
 #[derive(Clone, Debug, PartialEq, Eq)]
 pub struct UnboundDecl(pub(crate) SyntaxNode);
@@ -691,6 +827,20 @@ impl Decl {
             _ => panic!("not an unbound declaration"),
         }
     }
+
+    /// Define an ordering for declarations.
+    fn decl_index(&self) -> usize {
+        match self {
+            Self::Bound(b) => match b.ty().is_optional() {
+                true => 2,
+                false => 3,
+            },
+            Self::Unbound(u) => match u.ty().is_optional() {
+                true => 1,
+                false => 0,
+            },
+        }
+    }
 }
 
 impl AstNode for Decl {
@@ -722,6 +872,33 @@ impl AstNode for Decl {
     }
 }
 
+impl PartialOrd for Decl {
+    fn partial_cmp(&self, other: &Self) -> Option<std::cmp::Ordering> {
+        Some(self.cmp(other))
+    }
+}
+
+impl Ord for Decl {
+    fn cmp(&self, other: &Self) -> std::cmp::Ordering {
+        compare_decl(self, other)
+    }
+}
+
+/// Compare all variants of Decl.
+fn compare_decl(a: &Decl, b: &Decl) -> std::cmp::Ordering {
+    if (matches!(a, Decl::Bound(_))
+        && matches!(b, Decl::Bound(_))
+        && a.ty().is_optional() == b.ty().is_optional())
+        || (matches!(a, Decl::Unbound(_))
+            && matches!(b, Decl::Unbound(_))
+            && a.ty().is_optional() == b.ty().is_optional())
+    {
+        a.ty().cmp(&b.ty())
+    } else {
+        a.decl_index().cmp(&b.decl_index())
+    }
+}
+
 #[cfg(test)]
 mod test {
     use super::*;

wdl-lint/CHANGELOG.md

@@ -14,6 +14,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 * Added the `MissingMetas` and `MissingOutput` lint rules (#96).
 * 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)).
+* Added the `InputNotSorted` lint rule ([#100](https://github.com/stjude-rust-labs/wdl/pull/100)).
 
 ### Fixed
 

wdl-lint/RULES.md

@@ -14,6 +14,7 @@ be out of sync with released packages.
 | `ImportPlacement`                | Clarity                       | Ensures that imports are placed between the version statement and any document items. |
 | `ImportSort`                     | Clarity, Style                | Ensures that imports are sorted lexicographically.                                    |
 | `ImportWhitespace`               | Clarity, Style, Spacing       | Ensures that there is no extraneous whitespace between or within imports.             |
+| `InputNotSorted`                 | Style                         | Ensures that input declarations are sorted                 |
 | `MatchingParameterMeta`          | Completeness                  | Ensures that inputs have a matching entry in a `parameter_meta` section.              |
 | `MissingRuntime`                 | Completeness, Portability     | Ensures that tasks have a runtime section.                                            |
 | `MissingMetas`                   | Completeness, Clarity         | Ensures that tasks have both a meta and a parameter_meta section.                     |

wdl-lint/src/lib.rs

@@ -92,6 +92,7 @@ pub fn rules() -> Vec<Box<dyn Rule>> {
         Box::new(rules::MissingMetasRule),
         Box::new(rules::MissingOutputRule),
         Box::new(rules::ImportSortRule),
+        Box::new(rules::InputNotSortedRule),
         Box::new(rules::LineWidthRule::default()),
     ];
 

wdl-lint/src/rules.rs

@@ -6,6 +6,7 @@ mod ending_newline;
 mod import_placement;
 mod import_sort;
 mod import_whitespace;
+mod input_not_sorted;
 mod line_width;
 mod matching_parameter_meta;
 mod missing_metas;
@@ -24,6 +25,7 @@ pub use ending_newline::*;
 pub use import_placement::*;
 pub use import_sort::*;
 pub use import_whitespace::*;
+pub use input_not_sorted::*;
 pub use line_width::*;
 pub use matching_parameter_meta::*;
 pub use missing_metas::*;