Add in a prototype KCL linter (#2521)

* Add in a prototype KCL linter

This is a fork-and-replce of an experimental project I hacked up called
"kcl-vet", which was mostly the same code. This integrates kcl-vet into
the kcl_lib crate, which will let us use this from the zoo cli, as well
as via wasm in the lsp. this contains the intial integration with the
lsp, adding all lints as informational to start.

I need to go back and clean some of this up (and merge some of this back
into other parts of kcl_lib); but this has some pretty good progress
already.

Co-authored-by: jess@zoo.dev
Signed-off-by: Paul R. Tagliamonte <paul@zoo.dev>

* ty clippy :)

* add in a lint test

* add in some docstrings

* whoops

* sigh

* A snapshot a day keeps the bugs away! 📷🐛 (OS: ubuntu)

* uno reverse card

* A snapshot a day keeps the bugs away! 📷🐛 (OS: ubuntu)

* wtf stop it robot fuck

* A snapshot a day keeps the bugs away! 📷🐛 (OS: ubuntu)

* Revert "A snapshot a day keeps the bugs away! 📷🐛 (OS: ubuntu)"

This reverts commit 5b18f3c035.

* hurm

* try harder to type slower

* try harder? this all passes locally.

* try this now

* simplify, add debugging for trace

* fix enter use

* re-order again

* reorder a bit more

* enter

* ok fine no other enters?

* nerd

* wip

* move control of clearing to typescript

* move result out

* err check

* remove log

* remove clear

* remove add to diag

* THERE CAN BE ONLY ONE

* _err

* dedupe

* Revert "dedupe"

This reverts commit f66de88200.

* attempt to dedupe

* clear diagnostics on mock execute, too

* handle dupe diagnostics

* fmt

* dedupe tsc

* == vs ===

* fix dedupe

* return this to the wasm for now

* clear the map every go around

this is different than the old code isnce it won't republish

---------

Signed-off-by: Paul R. Tagliamonte <paul@zoo.dev>
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

.codespellrc

@@ -1,3 +1,3 @@
 [codespell]
-ignore-words-list: crate,everytime,inout,co-ordinate,ot,nwo,absolutey,atleast
+ignore-words-list: crate,everytime,inout,co-ordinate,ot,nwo,absolutey,atleast,ue
 skip: **/target,node_modules,build,**/Cargo.lock,./docs/kcl/*.md,./src-tauri/gen/schemas

e2e/playwright/flow-tests.spec.ts

@@ -421,8 +421,8 @@ test('if you write invalid kcl you get inlined errors', async ({ page }) => {
   // check no error to begin with
   await expect(page.locator('.cm-lint-marker-error')).not.toBeVisible()
 
-  /* add the following code to the editor (# error is not a valid line)
+  /* add the following code to the editor ($ error is not a valid line)
-    # error
+    $ error
     const topAng = 30
     const bottomAng = 25
    */
@@ -463,6 +463,8 @@ test('if you write invalid kcl you get inlined errors', async ({ page }) => {
   await page.keyboard.type("// Let's define the same thing twice")
   await page.keyboard.press('Enter')
   await page.keyboard.type('const topAng = 42')
+  await page.keyboard.press('ArrowLeft')
+  await page.keyboard.press('ArrowRight')
 
   await expect(page.locator('.cm-lint-marker-error')).toBeVisible()
   await expect(page.locator('.cm-lintRange.cm-lintRange-error')).toBeVisible()

src/editor/manager.ts

@@ -7,7 +7,11 @@ import { Selections, processCodeMirrorRanges, Selection } from 'lib/selections'
 import { undo, redo } from '@codemirror/commands'
 import { CommandBarMachineEvent } from 'machines/commandBarMachine'
 import { addLineHighlight } from './highlightextension'
-import { setDiagnostics, Diagnostic } from '@codemirror/lint'
+import { forEachDiagnostic, setDiagnostics, Diagnostic } from '@codemirror/lint'
+
+function diagnosticIsEqual(d1: Diagnostic, d2: Diagnostic): boolean {
+  return d1.from === d2.from && d1.to === d2.to && d1.message === d2.message
+}
 
 export default class EditorManager {
   private _editorView: EditorView | null = null
@@ -91,11 +95,38 @@ export default class EditorManager {
     }
   }
 
+  clearDiagnostics(): void {
+    if (!this.editorView) return
+    this.editorView.dispatch(setDiagnostics(this.editorView.state, []))
+  }
+
   setDiagnostics(diagnostics: Diagnostic[]): void {
     if (!this.editorView) return
     this.editorView.dispatch(setDiagnostics(this.editorView.state, diagnostics))
   }
 
+  addDiagnostics(diagnostics: Diagnostic[]): void {
+    if (!this.editorView) return
+
+    forEachDiagnostic(this.editorView.state, function (diag) {
+      diagnostics.push(diag)
+    })
+
+    const uniqueDiagnostics = new Set<Diagnostic>()
+    diagnostics.forEach((diagnostic) => {
+      for (const knownDiagnostic of uniqueDiagnostics.values()) {
+        if (diagnosticIsEqual(diagnostic, knownDiagnostic)) {
+          return
+        }
+      }
+      uniqueDiagnostics.add(diagnostic)
+    })
+
+    this.editorView.dispatch(
+      setDiagnostics(this.editorView.state, [...uniqueDiagnostics])
+    )
+  }
+
   undo() {
     if (this._editorView) {
       undo(this._editorView)

src/editor/plugins/lsp/plugin.ts

@@ -382,9 +382,14 @@ export class LanguageServerPlugin implements PluginValue {
     try {
       switch (notification.method) {
         case 'textDocument/publishDiagnostics':
-          //const params = notification.params as PublishDiagnosticsParams
+          console.log(
+            '[lsp] [window/publishDiagnostics]',
+            this.client.getName(),
+            notification.params
+          )
+          const params = notification.params as PublishDiagnosticsParams
           // this is sometimes slower than our actual typing.
-          //this.processDiagnostics(params)
+          this.processDiagnostics(params)
           break
         case 'window/logMessage':
           console.log(

src/lang/KclSingleton.ts

@@ -89,9 +89,10 @@ export class KclManager {
     return this._kclErrors
   }
   set kclErrors(kclErrors) {
+    console.log('[lsp] not lsp, actually typescript: ', kclErrors)
     this._kclErrors = kclErrors
     let diagnostics = kclErrorsToDiagnostics(kclErrors)
-    editorManager.setDiagnostics(diagnostics)
+    editorManager.addDiagnostics(diagnostics)
     this._kclErrorsCallBack(kclErrors)
   }
 
@@ -185,6 +186,11 @@ export class KclManager {
     const currentExecutionId = executionId || Date.now()
     this._cancelTokens.set(currentExecutionId, false)
 
+    // here we're going to clear diagnostics since we're the first
+    // one in. We're the only location where diagnostics are cleared;
+    // everything from here on out should be *appending*.
+    editorManager.clearDiagnostics()
+
     this.isExecuting = true
     await this.ensureWasmInit()
     const { logs, errors, programMemory } = await executeAst({
@@ -234,6 +240,7 @@ export class KclManager {
     } = { updates: 'none' }
   ) {
     await this.ensureWasmInit()
+
     const newCode = recast(ast)
     const newAst = this.safeParse(newCode)
     if (!newAst) return
@@ -243,6 +250,11 @@ export class KclManager {
     await this?.engineCommandManager?.waitForReady
     this._ast = { ...newAst }
 
+    // here we're going to clear diagnostics since we're the first
+    // one in. We're the only location where diagnostics are cleared;
+    // everything from here on out should be *appending*.
+    editorManager.clearDiagnostics()
+
     const { logs, errors, programMemory } = await executeAst({
       ast: newAst,
       engineCommandManager: this.engineCommandManager,

src/wasm-lib/kcl/src/errors.rs

@@ -2,7 +2,7 @@ use serde::{Deserialize, Serialize};
 use thiserror::Error;
 use tower_lsp::lsp_types::{Diagnostic, DiagnosticSeverity};
 
-use crate::executor::SourceRange;
+use crate::{executor::SourceRange, lsp::IntoDiagnostic};
 
 #[derive(Error, Debug, Serialize, Deserialize, ts_rs::TS, Clone, PartialEq, Eq)]
 #[ts(export)]
@@ -42,19 +42,9 @@ pub struct KclErrorDetails {
 }
 
 impl KclError {
-    /// Get the error message, line and column from the error and input code.
+    /// Get the error message.
-    pub fn get_message_line_column(&self, input: &str) -> (String, Option<usize>, Option<usize>) {
+    pub fn get_message(&self) -> String {
-        // Calculate the line and column of the error from the source range.
+        format!("{}: {}", self.error_type(), self.message())
-        let (line, column) = if let Some(range) = self.source_ranges().first() {
-            let line = input[..range.0[0]].lines().count();
-            let column = input[..range.0[0]].lines().last().map(|l| l.len()).unwrap_or_default();
-
-            (Some(line), Some(column))
-        } else {
-            (None, None)
-        };
-
-        (format!("{}: {}", self.error_type(), self.message()), line, column)
     }
 
     pub fn error_type(&self) -> &'static str {
@@ -106,24 +96,6 @@ impl KclError {
         }
     }
 
-    pub fn to_lsp_diagnostic(&self, code: &str) -> Diagnostic {
-        let (message, _, _) = self.get_message_line_column(code);
-        let source_ranges = self.source_ranges();
-
-        Diagnostic {
-            range: source_ranges.first().map(|r| r.to_lsp_range(code)).unwrap_or_default(),
-            severity: Some(DiagnosticSeverity::ERROR),
-            code: None,
-            // TODO: this is neat we can pass a URL to a help page here for this specific error.
-            code_description: None,
-            source: Some("kcl".to_string()),
-            message,
-            related_information: None,
-            tags: None,
-            data: None,
-        }
-    }
-
     pub fn override_source_ranges(&self, source_ranges: Vec<SourceRange>) -> Self {
         let mut new = self.clone();
         match &mut new {
@@ -163,6 +135,26 @@ impl KclError {
     }
 }
 
+impl IntoDiagnostic for KclError {
+    fn to_lsp_diagnostic(&self, code: &str) -> Diagnostic {
+        let message = self.get_message();
+        let source_ranges = self.source_ranges();
+
+        Diagnostic {
+            range: source_ranges.first().map(|r| r.to_lsp_range(code)).unwrap_or_default(),
+            severity: Some(DiagnosticSeverity::ERROR),
+            code: None,
+            // TODO: this is neat we can pass a URL to a help page here for this specific error.
+            code_description: None,
+            source: Some("kcl".to_string()),
+            message,
+            related_information: None,
+            tags: None,
+            data: None,
+        }
+    }
+}
+
 /// This is different than to_string() in that it will serialize the Error
 /// the struct as JSON so we can deserialize it on the js side.
 impl From<KclError> for String {

src/wasm-lib/kcl/src/lib.rs

@@ -11,6 +11,7 @@ pub mod engine;
 pub mod errors;
 pub mod executor;
 pub mod fs;
+pub mod lint;
 pub mod lsp;
 pub mod parser;
 pub mod settings;

src/wasm-lib/kcl/src/lint/ast_node.rs

@@ -0,0 +1,64 @@
+use crate::ast::types;
+
+/// The "Node" type wraps all the AST elements we're able to find in a KCL
+/// file. Tokens we walk through will be one of these.
+#[derive(Clone, Debug)]
+pub enum Node<'a> {
+    Program(&'a types::Program),
+
+    ExpressionStatement(&'a types::ExpressionStatement),
+    VariableDeclaration(&'a types::VariableDeclaration),
+    ReturnStatement(&'a types::ReturnStatement),
+
+    VariableDeclarator(&'a types::VariableDeclarator),
+
+    Literal(&'a types::Literal),
+    Identifier(&'a types::Identifier),
+    BinaryExpression(&'a types::BinaryExpression),
+    FunctionExpression(&'a types::FunctionExpression),
+    CallExpression(&'a types::CallExpression),
+    PipeExpression(&'a types::PipeExpression),
+    PipeSubstitution(&'a types::PipeSubstitution),
+    ArrayExpression(&'a types::ArrayExpression),
+    ObjectExpression(&'a types::ObjectExpression),
+    MemberExpression(&'a types::MemberExpression),
+    UnaryExpression(&'a types::UnaryExpression),
+
+    Parameter(&'a types::Parameter),
+
+    ObjectProperty(&'a types::ObjectProperty),
+
+    MemberObject(&'a types::MemberObject),
+    LiteralIdentifier(&'a types::LiteralIdentifier),
+}
+
+macro_rules! impl_from {
+    ($node:ident, $t: ident) => {
+        impl<'a> From<&'a types::$t> for Node<'a> {
+            fn from(v: &'a types::$t) -> Self {
+                Node::$t(v)
+            }
+        }
+    };
+}
+
+impl_from!(Node, Program);
+impl_from!(Node, ExpressionStatement);
+impl_from!(Node, VariableDeclaration);
+impl_from!(Node, ReturnStatement);
+impl_from!(Node, VariableDeclarator);
+impl_from!(Node, Literal);
+impl_from!(Node, Identifier);
+impl_from!(Node, BinaryExpression);
+impl_from!(Node, FunctionExpression);
+impl_from!(Node, CallExpression);
+impl_from!(Node, PipeExpression);
+impl_from!(Node, PipeSubstitution);
+impl_from!(Node, ArrayExpression);
+impl_from!(Node, ObjectExpression);
+impl_from!(Node, MemberExpression);
+impl_from!(Node, UnaryExpression);
+impl_from!(Node, Parameter);
+impl_from!(Node, ObjectProperty);
+impl_from!(Node, MemberObject);
+impl_from!(Node, LiteralIdentifier);

src/wasm-lib/kcl/src/lint/ast_walk.rs

@@ -0,0 +1,233 @@
+use super::Node;
+use crate::ast::types::{
+    BinaryPart, BodyItem, LiteralIdentifier, MemberExpression, MemberObject, ObjectExpression, ObjectProperty,
+    Parameter, Program, UnaryExpression, Value, VariableDeclarator,
+};
+use anyhow::Result;
+
+/// Walker is implemented by things that are able to walk an AST tree to
+/// produce lints. This trait is implemented automatically for a few of the
+/// common types, but can be manually implemented too.
+pub trait Walker<'a> {
+    /// Walk will visit every element of the AST.
+    fn walk(&self, n: Node<'a>) -> Result<bool>;
+}
+
+impl<'a, FnT> Walker<'a> for FnT
+where
+    FnT: Fn(Node<'a>) -> Result<bool>,
+{
+    fn walk(&self, n: Node<'a>) -> Result<bool> {
+        self(n)
+    }
+}
+
+/// Run the Walker against all [Node]s in a [Program].
+pub fn walk<'a, WalkT>(prog: &'a Program, f: &WalkT) -> Result<()>
+where
+    WalkT: Walker<'a>,
+{
+    f.walk(prog.into())?;
+
+    for bi in &prog.body {
+        walk_body_item(bi, f)?;
+    }
+    Ok(())
+}
+
+fn walk_variable_declarator<'a, WalkT>(node: &'a VariableDeclarator, f: &WalkT) -> Result<()>
+where
+    WalkT: Walker<'a>,
+{
+    f.walk(node.into())?;
+    f.walk((&node.id).into())?;
+    walk_value(&node.init, f)?;
+    Ok(())
+}
+
+fn walk_parameter<'a, WalkT>(node: &'a Parameter, f: &WalkT) -> Result<()>
+where
+    WalkT: Walker<'a>,
+{
+    f.walk(node.into())?;
+    f.walk((&node.identifier).into())?;
+    Ok(())
+}
+
+fn walk_member_object<'a, WalkT>(node: &'a MemberObject, f: &WalkT) -> Result<()>
+where
+    WalkT: Walker<'a>,
+{
+    f.walk(node.into())?;
+    Ok(())
+}
+
+fn walk_literal_identifier<'a, WalkT>(node: &'a LiteralIdentifier, f: &WalkT) -> Result<()>
+where
+    WalkT: Walker<'a>,
+{
+    f.walk(node.into())?;
+    Ok(())
+}
+
+fn walk_member_expression<'a, WalkT>(node: &'a MemberExpression, f: &WalkT) -> Result<()>
+where
+    WalkT: Walker<'a>,
+{
+    f.walk(node.into())?;
+
+    walk_member_object(&node.object, f)?;
+    walk_literal_identifier(&node.property, f)?;
+
+    Ok(())
+}
+
+fn walk_binary_part<'a, WalkT>(node: &'a BinaryPart, f: &WalkT) -> Result<()>
+where
+    WalkT: Walker<'a>,
+{
+    match node {
+        BinaryPart::Literal(lit) => f.walk(lit.as_ref().into())?,
+        BinaryPart::Identifier(id) => f.walk(id.as_ref().into())?,
+        BinaryPart::BinaryExpression(be) => f.walk(be.as_ref().into())?,
+        BinaryPart::CallExpression(ce) => f.walk(ce.as_ref().into())?,
+        BinaryPart::UnaryExpression(ue) => {
+            walk_unary_expression(ue, f)?;
+            true
+        }
+        BinaryPart::MemberExpression(me) => {
+            walk_member_expression(me, f)?;
+            true
+        }
+    };
+
+    Ok(())
+}
+
+fn walk_value<'a, WalkT>(node: &'a Value, f: &WalkT) -> Result<()>
+where
+    WalkT: Walker<'a>,
+{
+    match node {
+        Value::Literal(lit) => {
+            f.walk(lit.as_ref().into())?;
+        }
+
+        Value::Identifier(id) => {
+            // sometimes there's a bare Identifier without a Value::Identifier.
+            f.walk(id.as_ref().into())?;
+        }
+
+        Value::BinaryExpression(be) => {
+            f.walk(be.as_ref().into())?;
+
+            walk_binary_part(&be.left, f)?;
+            walk_binary_part(&be.right, f)?;
+        }
+        Value::FunctionExpression(fe) => {
+            f.walk(fe.as_ref().into())?;
+
+            for arg in &fe.params {
+                walk_parameter(arg, f)?;
+            }
+            walk(&fe.body, f)?;
+        }
+        Value::CallExpression(ce) => {
+            f.walk(ce.as_ref().into())?;
+            f.walk((&ce.callee).into())?;
+            for e in &ce.arguments {
+                walk_value::<WalkT>(e, f)?;
+            }
+        }
+        Value::PipeExpression(pe) => {
+            f.walk(pe.as_ref().into())?;
+
+            for e in &pe.body {
+                walk_value::<WalkT>(e, f)?;
+            }
+        }
+        Value::PipeSubstitution(ps) => {
+            f.walk(ps.as_ref().into())?;
+        }
+        Value::ArrayExpression(ae) => {
+            f.walk(ae.as_ref().into())?;
+            for e in &ae.elements {
+                walk_value::<WalkT>(e, f)?;
+            }
+        }
+        Value::ObjectExpression(oe) => {
+            walk_object_expression(oe, f)?;
+        }
+        Value::MemberExpression(me) => {
+            walk_member_expression(me, f)?;
+        }
+        Value::UnaryExpression(ue) => {
+            walk_unary_expression(ue, f)?;
+        }
+        _ => {
+            println!("{:?}", node);
+            unimplemented!()
+        }
+    }
+
+    Ok(())
+}
+
+/// Walk through an [ObjectProperty].
+fn walk_object_property<'a, WalkT>(node: &'a ObjectProperty, f: &WalkT) -> Result<()>
+where
+    WalkT: Walker<'a>,
+{
+    f.walk(node.into())?;
+    walk_value(&node.value, f)?;
+    Ok(())
+}
+
+/// Walk through an [ObjectExpression].
+fn walk_object_expression<'a, WalkT>(node: &'a ObjectExpression, f: &WalkT) -> Result<()>
+where
+    WalkT: Walker<'a>,
+{
+    f.walk(node.into())?;
+    for prop in &node.properties {
+        walk_object_property(prop, f)?;
+    }
+    Ok(())
+}
+
+/// walk through an [UnaryExpression].
+fn walk_unary_expression<'a, WalkT>(node: &'a UnaryExpression, f: &WalkT) -> Result<()>
+where
+    WalkT: Walker<'a>,
+{
+    f.walk(node.into())?;
+    walk_binary_part(&node.argument, f)?;
+    Ok(())
+}
+
+/// walk through a [BodyItem].
+fn walk_body_item<'a, WalkT>(node: &'a BodyItem, f: &WalkT) -> Result<()>
+where
+    WalkT: Walker<'a>,
+{
+    // We don't walk a BodyItem since it's an enum itself.
+
+    match node {
+        BodyItem::ExpressionStatement(xs) => {
+            f.walk(xs.into())?;
+            walk_value(&xs.expression, f)?;
+        }
+        BodyItem::VariableDeclaration(vd) => {
+            f.walk(vd.into())?;
+            for dec in &vd.declarations {
+                walk_variable_declarator(dec, f)?;
+            }
+        }
+        BodyItem::ReturnStatement(rs) => {
+            f.walk(rs.into())?;
+            walk_value(&rs.argument, f)?;
+        }
+    }
+
+    Ok(())
+}

src/wasm-lib/kcl/src/lint/checks/camel_case.rs

@@ -0,0 +1,131 @@
+use crate::{
+    ast::types::VariableDeclarator,
+    executor::SourceRange,
+    lint::{
+        rule::{def_finding, Discovered, Finding},
+        Node,
+    },
+};
+
+use anyhow::Result;
+
+def_finding!(
+    Z0001,
+    "Identifiers must be lowerCamelCase",
+    "\
+By convention, variable names are lowerCamelCase, not snake_case, kebab-case,
+nor CammelCase. 🐪
+
+For instance, a good identifier for the variable representing 'box height'
+would be 'boxHeight', not 'BOX_HEIGHT', 'box_height' nor 'BoxHeight'. For
+more information there's a pretty good Wikipedia page at
+
+https://en.wikipedia.org/wiki/Camel_case
+"
+);
+
+fn lint_lower_camel_case(decl: &VariableDeclarator) -> Result<Vec<Discovered>> {
+    let mut findings = vec![];
+    let ident = &decl.id;
+    let name = &ident.name;
+
+    if !name.chars().next().unwrap().is_lowercase() {
+        findings.push(Z0001.at(format!("found '{}'", name), SourceRange::new(ident.start, ident.end)));
+        return Ok(findings);
+    }
+
+    if name.contains('-') || name.contains('_') {
+        findings.push(Z0001.at(format!("found '{}'", name), SourceRange::new(ident.start, ident.end)));
+        return Ok(findings);
+    }
+
+    Ok(findings)
+}
+
+pub fn lint_variables(decl: Node) -> Result<Vec<Discovered>> {
+    let Node::VariableDeclaration(decl) = decl else {
+        return Ok(vec![]);
+    };
+
+    Ok(decl
+        .declarations
+        .iter()
+        .flat_map(|v| lint_lower_camel_case(v).unwrap_or_default())
+        .collect())
+}
+
+#[cfg(test)]
+mod tests {
+    use super::{lint_variables, Z0001};
+    use crate::lint::rule::{assert_finding, test_finding, test_no_finding};
+
+    #[test]
+    fn z0001_const() {
+        assert_finding!(lint_variables, Z0001, "const Thickness = 0.5");
+        assert_finding!(lint_variables, Z0001, "const THICKNESS = 0.5");
+        assert_finding!(lint_variables, Z0001, "const THICC_NES = 0.5");
+        assert_finding!(lint_variables, Z0001, "const thicc_nes = 0.5");
+    }
+
+    test_finding!(z0001_full_bad, lint_variables, Z0001, "\
+// Define constants
+const pipeLength = 40
+const pipeSmallDia = 10
+const pipeLargeDia = 20
+const thickness = 0.5
+
+// Create the sketch to be revolved around the y-axis. Use the small diameter, large diameter, length, and thickness to define the sketch.
+const Part001 = startSketchOn('XY')
+  |> startProfileAt([pipeLargeDia - (thickness / 2), 38], %)
+  |> line([thickness, 0], %)
+  |> line([0, -1], %)
+  |> angledLineToX({
+       angle: 60,
+       to: pipeSmallDia + thickness
+     }, %)
+  |> line([0, -pipeLength], %)
+  |> angledLineToX({
+       angle: -60,
+       to: pipeLargeDia + thickness
+     }, %)
+  |> line([0, -1], %)
+  |> line([-thickness, 0], %)
+  |> line([0, 1], %)
+  |> angledLineToX({ angle: 120, to: pipeSmallDia }, %)
+  |> line([0, pipeLength], %)
+  |> angledLineToX({ angle: 60, to: pipeLargeDia }, %)
+  |> close(%)
+  |> revolve({ axis: 'y' }, %)
+");
+
+    test_no_finding!(z0001_full_good, lint_variables, Z0001, "\
+// Define constants
+const pipeLength = 40
+const pipeSmallDia = 10
+const pipeLargeDia = 20
+const thickness = 0.5
+
+// Create the sketch to be revolved around the y-axis. Use the small diameter, large diameter, length, and thickness to define the sketch.
+const part001 = startSketchOn('XY')
+  |> startProfileAt([pipeLargeDia - (thickness / 2), 38], %)
+  |> line([thickness, 0], %)
+  |> line([0, -1], %)
+  |> angledLineToX({
+       angle: 60,
+       to: pipeSmallDia + thickness
+     }, %)
+  |> line([0, -pipeLength], %)
+  |> angledLineToX({
+       angle: -60,
+       to: pipeLargeDia + thickness
+     }, %)
+  |> line([0, -1], %)
+  |> line([-thickness, 0], %)
+  |> line([0, 1], %)
+  |> angledLineToX({ angle: 120, to: pipeSmallDia }, %)
+  |> line([0, pipeLength], %)
+  |> angledLineToX({ angle: 60, to: pipeLargeDia }, %)
+  |> close(%)
+  |> revolve({ axis: 'y' }, %)
+");
+}

src/wasm-lib/kcl/src/lint/checks/mod.rs

@@ -0,0 +1,4 @@
+mod camel_case;
+
+#[allow(unused_imports)]
+pub use camel_case::{lint_variables, Z0001};

src/wasm-lib/kcl/src/lint/mod.rs

@@ -0,0 +1,9 @@
+mod ast_node;
+mod ast_walk;
+pub mod checks;
+mod rule;
+
+pub use ast_node::Node;
+pub use ast_walk::walk;
+// pub(crate) use rule::{def_finding, finding};
+pub use rule::{lint, Discovered, Finding};

src/wasm-lib/kcl/src/lint/rule.rs

@@ -0,0 +1,180 @@
+use super::{walk, Node};
+use crate::{ast::types::Program, executor::SourceRange, lsp::IntoDiagnostic};
+use anyhow::Result;
+use std::sync::{Arc, Mutex};
+use tower_lsp::lsp_types::{Diagnostic, DiagnosticSeverity};
+
+/// Check the provided AST for any found rule violations.
+///
+/// The Rule trait is automatically implemented for a few other types,
+/// but it can also be manually implemented as required.
+pub trait Rule<'a> {
+    /// Check the AST at this specific node for any Finding(s).
+    fn check(&self, node: Node<'a>) -> Result<Vec<Discovered>>;
+}
+
+impl<'a, FnT> Rule<'a> for FnT
+where
+    FnT: Fn(Node<'a>) -> Result<Vec<Discovered>>,
+{
+    fn check(&self, n: Node<'a>) -> Result<Vec<Discovered>> {
+        self(n)
+    }
+}
+
+/// Specific discovered lint rule Violation of a particular Finding.
+#[derive(Clone, Debug)]
+pub struct Discovered {
+    /// Zoo Lint Finding information.
+    pub finding: Finding,
+
+    /// Further information about the specific finding.
+    pub description: String,
+
+    /// Source code location.
+    pub pos: SourceRange,
+
+    /// Is this discovered issue overridden by the programmer?
+    pub overridden: bool,
+}
+
+impl IntoDiagnostic for Discovered {
+    fn to_lsp_diagnostic(&self, code: &str) -> Diagnostic {
+        let message = self.finding.title.to_owned();
+        let source_range = self.pos;
+
+        Diagnostic {
+            range: source_range.to_lsp_range(code),
+            severity: Some(DiagnosticSeverity::INFORMATION),
+            code: None,
+            // TODO: this is neat we can pass a URL to a help page here for this specific error.
+            code_description: None,
+            source: Some("lint".to_string()),
+            message,
+            related_information: None,
+            tags: None,
+            data: None,
+        }
+    }
+}
+
+/// Abstract lint problem type.
+#[derive(Clone, Debug, PartialEq)]
+pub struct Finding {
+    /// Unique identifier for this particular issue.
+    pub code: &'static str,
+
+    /// Short one-line description of this issue.
+    pub title: &'static str,
+
+    /// Long human-readable description of this issue.
+    pub description: &'static str,
+
+    /// Is this discovered issue experimental?
+    pub experimental: bool,
+}
+
+impl Finding {
+    /// Create a new Discovered finding at the specific Position.
+    pub fn at(&self, description: String, pos: SourceRange) -> Discovered {
+        Discovered {
+            description,
+            finding: self.clone(),
+            pos,
+            overridden: false,
+        }
+    }
+}
+
+macro_rules! def_finding {
+    ( $code:ident, $title:expr, $description:expr ) => {
+        /// Generated Finding
+        pub const $code: Finding = $crate::lint::rule::finding!($code, $title, $description);
+    };
+}
+pub(crate) use def_finding;
+
+macro_rules! finding {
+    ( $code:ident, $title:expr, $description:expr ) => {
+        $crate::lint::rule::Finding {
+            code: stringify!($code),
+            title: $title,
+            description: $description,
+            experimental: false,
+        }
+    };
+}
+pub(crate) use finding;
+
+#[cfg(test)]
+pub(crate) use test::{assert_finding, assert_no_finding, test_finding, test_no_finding};
+
+/// Check the provided Program for any Findings.
+pub fn lint<'a, RuleT>(prog: &'a Program, rule: RuleT) -> Result<Vec<Discovered>>
+where
+    RuleT: Rule<'a>,
+{
+    let v = Arc::new(Mutex::new(vec![]));
+    walk(prog, &|node: Node<'a>| {
+        let mut findings = v.lock().map_err(|_| anyhow::anyhow!("mutex"))?;
+        findings.append(&mut rule.check(node)?);
+        Ok(true)
+    })?;
+    let x = v.lock().unwrap();
+    Ok(x.clone())
+}
+
+#[cfg(test)]
+mod test {
+
+    macro_rules! assert_no_finding {
+        ( $check:expr, $finding:expr, $kcl:expr ) => {
+            let tokens = $crate::token::lexer($kcl).unwrap();
+            let parser = $crate::parser::Parser::new(tokens);
+            let prog = parser.ast().unwrap();
+            for discovered_finding in $crate::lint::lint(&prog, $check).unwrap() {
+                if discovered_finding.finding == $finding {
+                    assert!(false, "Finding {:?} was emitted", $finding.code);
+                }
+            }
+        };
+    }
+
+    macro_rules! assert_finding {
+        ( $check:expr, $finding:expr, $kcl:expr ) => {
+            let tokens = $crate::token::lexer($kcl).unwrap();
+            let parser = $crate::parser::Parser::new(tokens);
+            let prog = parser.ast().unwrap();
+
+            for discovered_finding in $crate::lint::lint(&prog, $check).unwrap() {
+                if discovered_finding.finding == $finding {
+                    return;
+                }
+            }
+            assert!(false, "Finding {:?} was not emitted", $finding.code);
+        };
+    }
+
+    macro_rules! test_finding {
+        ( $name:ident, $check:expr, $finding:expr, $kcl:expr ) => {
+            #[test]
+            fn $name() {
+                $crate::lint::rule::assert_finding!($check, $finding, $kcl);
+            }
+        };
+    }
+
+    macro_rules! test_no_finding {
+        ( $name:ident, $check:expr, $finding:expr, $kcl:expr ) => {
+            #[test]
+            fn $name() {
+                $crate::lint::rule::assert_no_finding!($check, $finding, $kcl);
+            }
+        };
+    }
+
+    pub(crate) use assert_finding;
+    pub(crate) use assert_no_finding;
+    pub(crate) use test_finding;
+    pub(crate) use test_no_finding;
+}

src/wasm-lib/kcl/src/lsp/kcl/mod.rs

@@ -36,9 +36,9 @@ use tower_lsp::{
 use super::backend::{InnerHandle, UpdateHandle};
 use crate::{
     ast::types::VariableKind,
-    errors::KclError,
     executor::SourceRange,
-    lsp::{backend::Backend as _, safemap::SafeMap},
+    lint::{checks, lint},
+    lsp::{backend::Backend as _, safemap::SafeMap, util::IntoDiagnostic},
     parser::PIPE_OPERATOR,
 };
 
@@ -166,6 +166,7 @@ impl crate::lsp::backend::Backend for Backend {
     }
 
     async fn inner_on_change(&self, params: TextDocumentItem, force: bool) {
+        self.clear_diagnostics_map(&params.uri).await;
         // We already updated the code map in the shared backend.
 
         // Lets update the tokens.
@@ -251,14 +252,14 @@ impl crate::lsp::backend::Backend for Backend {
         // Execute the code if we have an executor context.
         // This function automatically executes if we should & updates the diagnostics if we got
         // errors.
-        let result = self.execute(&params, ast).await;
+        if self.execute(&params, ast.clone()).await.is_err() {
-        if result.is_err() {
+            // if there was an issue, let's bail and avoid trying to lint.
-            // We return early because we got errors, and we don't want to clear the diagnostics.
             return;
         }
 
-        // Lets update the diagnostics, since we got no errors.
+        for discovered_finding in lint(&ast, checks::lint_variables).into_iter().flatten() {
-        self.clear_diagnostics(&params.uri).await;
+            self.add_to_diagnostics(&params, discovered_finding).await;
+        }
     }
 }
 
@@ -356,30 +357,7 @@ impl Backend {
             .await;
     }
 
-    async fn add_to_diagnostics(&self, params: &TextDocumentItem, err: KclError) {
+    async fn clear_diagnostics_map(&self, uri: &url::Url) {
-        let diagnostic = err.to_lsp_diagnostic(&params.text);
-        // We got errors, update the diagnostics.
-        self.diagnostics_map
-            .insert(
-                params.uri.to_string(),
-                DocumentDiagnosticReport::Full(RelatedFullDocumentDiagnosticReport {
-                    related_documents: None,
-                    full_document_diagnostic_report: FullDocumentDiagnosticReport {
-                        result_id: None,
-                        items: vec![diagnostic.clone()],
-                    },
-                }),
-            )
-            .await;
-
-        // Publish the diagnostic.
-        // If the client supports it.
-        self.client
-            .publish_diagnostics(params.uri.clone(), vec![diagnostic], None)
-            .await;
-    }
-
-    async fn clear_diagnostics(&self, uri: &url::Url) {
         self.diagnostics_map
             .insert(
                 uri.to_string(),
@@ -392,10 +370,43 @@ impl Backend {
                 }),
             )
             .await;
+    }
 
-        // Publish the diagnostic, we reset it here so the client knows the code compiles now.
+    async fn add_to_diagnostics<DiagT: IntoDiagnostic + std::fmt::Debug>(
-        // If the client supports it.
+        &self,
-        self.client.publish_diagnostics(uri.clone(), vec![], None).await;
+        params: &TextDocumentItem,
+        diagnostic: DiagT,
+    ) {
+        self.client
+            .log_message(MessageType::INFO, format!("adding {:?} to diag", diagnostic))
+            .await;
+
+        let diagnostic = diagnostic.to_lsp_diagnostic(&params.text);
+
+        let DocumentDiagnosticReport::Full(mut report) = self
+            .diagnostics_map
+            .get(params.uri.clone().as_str())
+            .await
+            .unwrap_or(DocumentDiagnosticReport::Full(RelatedFullDocumentDiagnosticReport {
+                related_documents: None,
+                full_document_diagnostic_report: FullDocumentDiagnosticReport {
+                    result_id: None,
+                    items: vec![],
+                },
+            }))
+        else {
+            unreachable!();
+        };
+
+        report.full_document_diagnostic_report.items.push(diagnostic);
+
+        self.diagnostics_map
+            .insert(params.uri.to_string(), DocumentDiagnosticReport::Full(report.clone()))
+            .await;
+
+        self.client
+            .publish_diagnostics(params.uri.clone(), report.full_document_diagnostic_report.items, None)
+            .await;
     }
 
     async fn execute(&self, params: &TextDocumentItem, ast: crate::ast::types::Program) -> Result<()> {

src/wasm-lib/kcl/src/lsp/mod.rs

@@ -7,3 +7,5 @@ mod safemap;
 #[cfg(test)]
 mod tests;
 pub mod util;
+
+pub use util::IntoDiagnostic;

src/wasm-lib/kcl/src/lsp/tests.rs

@@ -1498,6 +1498,53 @@ async fn test_kcl_lsp_diagnostic_has_errors() {
     }
 }
 
+#[tokio::test(flavor = "multi_thread")]
+async fn test_kcl_lsp_diagnostic_has_lints() {
+    let server = kcl_lsp_server(false).await.unwrap();
+
+    // Send open file.
+    server
+        .did_open(tower_lsp::lsp_types::DidOpenTextDocumentParams {
+            text_document: tower_lsp::lsp_types::TextDocumentItem {
+                uri: "file:///testlint.kcl".try_into().unwrap(),
+                language_id: "kcl".to_string(),
+                version: 1,
+                text: r#"let THING = 10"#.to_string(),
+            },
+        })
+        .await;
+    server.wait_on_handle().await;
+
+    // Send diagnostics request.
+    let diagnostics = server
+        .diagnostic(tower_lsp::lsp_types::DocumentDiagnosticParams {
+            text_document: tower_lsp::lsp_types::TextDocumentIdentifier {
+                uri: "file:///testlint.kcl".try_into().unwrap(),
+            },
+            partial_result_params: Default::default(),
+            work_done_progress_params: Default::default(),
+            identifier: None,
+            previous_result_id: None,
+        })
+        .await
+        .unwrap();
+
+    // Check the diagnostics.
+    if let tower_lsp::lsp_types::DocumentDiagnosticReportResult::Report(diagnostics) = diagnostics {
+        if let tower_lsp::lsp_types::DocumentDiagnosticReport::Full(diagnostics) = diagnostics {
+            assert_eq!(diagnostics.full_document_diagnostic_report.items.len(), 1);
+            assert_eq!(
+                diagnostics.full_document_diagnostic_report.items[0].message,
+                "Identifiers must be lowerCamelCase"
+            );
+        } else {
+            panic!("Expected full diagnostics");
+        }
+    } else {
+        panic!("Expected diagnostics");
+    }
+}
+
 #[tokio::test(flavor = "multi_thread")]
 async fn test_copilot_lsp_set_editor_info() {
     let server = copilot_lsp_server().await.unwrap();

src/wasm-lib/kcl/src/lsp/util.rs

@@ -1,7 +1,7 @@
 //! Utility functions for working with ropes and positions.
 
 use ropey::Rope;
-use tower_lsp::lsp_types::Position;
+use tower_lsp::lsp_types::{Diagnostic, Position};
 
 pub fn position_to_offset(position: Position, rope: &Rope) -> Option<usize> {
     Some(rope.try_line_to_char(position.line as usize).ok()? + position.character as usize)
@@ -31,3 +31,10 @@ pub fn get_line_before(pos: Position, rope: &Rope) -> Option<String> {
     let line_start = offset - char_offset;
     Some(rope.slice(line_start..offset).to_string())
 }
+
+/// Convert an object into a [lsp_types::Diagnostic] given the
+/// [TextDocumentItem]'s `.text` field.
+pub trait IntoDiagnostic {
+    /// Convert the traited object to a [lsp_types::Diagnostic].
+    fn to_lsp_diagnostic(&self, text: &str) -> Diagnostic;
+}