Merge pull request #56 from anweiss/lsp

Comment formatting fixes

.devcontainer/devcontainer.json

@@ -1,11 +1,7 @@
 {
   "name": "Rust",
   "dockerFile": "Dockerfile",
-  "runArgs": [
-    "--cap-add=SYS_PTRACE",
-    "--security-opt",
-    "seccomp=unconfined"
-  ],
+  "runArgs": ["--cap-add=SYS_PTRACE", "--security-opt", "seccomp=unconfined"],
   // Set *default* container specific settings.json values on container create.
   "settings": {
     "terminal.integrated.shell.linux": "/bin/bash",
@@ -17,11 +13,12 @@
     "matklad.rust-analyzer",
     "bungcip.better-toml",
     "vadimcn.vscode-lldb",
+    "serayuzgur.crates"
   ],
   // Use 'forwardPorts' to make a list of ports inside the container available locally.
   // "forwardPorts": [],
   // Use 'postCreateCommand' to run commands after the container is created.
   "postCreateCommand": "cargo build",
   // Uncomment to connect as a non-root user. See https://aka.ms/vscode-remote/containers/non-root.
-  "remoteUser": "vscode",
-}
+  "remoteUser": "vscode"
+}

.github/workflows/publish.yml

@@ -26,7 +26,7 @@ jobs:
       - name: Build CLI
         if: matrix.os == 'ubuntu-latest'
         run: |
-          cargo b --release --bin cddl
+          cargo b --release --bin cli
           cp target/release/cddl cddl-linux-amd64
           chmod +x cddl-linux-amd64
           tar -czvf cddl-linux-amd64.tar.gz cddl-linux-amd64
@@ -35,7 +35,7 @@ jobs:
         if: matrix.os == 'macOS-latest'
         run: |
           source $HOME/.cargo/env
-          cargo b --release --bin cddl
+          cargo b --release --bin cli
           cp target/release/cddl cddl-darwin-amd64
           chmod +x cddl-darwin-amd64
           zip cddl-darwin-amd64.zip cddl-darwin-amd64
@@ -44,7 +44,7 @@ jobs:
         if: matrix.os == 'windows-latest'
         shell: powershell
         run: |
-          cargo b --release --bin cddl
+          cargo b --release --bin cli
           cp target\release\cddl.exe cddl-windows-amd64.exe
           Compress-Archive -Path .\cddl-windows-amd64.exe -DestinationPath cddl-windows-amd64.zip
 

Cargo.toml

@@ -5,7 +5,7 @@ repository = "https://github.com/anweiss/cddl"
 homepage = "https://cddl.anweiss.tech"
 categories = ["parser-implementations", "encoding", "development-tools", "wasm"]
 license = "MIT"
-version = "0.7.2"
+version = "0.7.3"
 authors = ["Andrew Weiss <[email protected]>"]
 readme = "README.md"
 edition = "2018"
@@ -50,14 +50,16 @@ lsp = ["std"]
 nightly = ["uriparse"]
 
 [[bin]]
-name = "cddl"
+name = "cli"
 required-features = ["std"]
-path = "src/bin/cddl.rs"
+path = "src/bin/cli.rs"
+test = false
 
 [[bin]]
 name = "repl"
 required-features = ["std"]
 path = "src/bin/repl.rs"
+test = false
 
 [[test]]
 name = "cddl"

Dockerfile

@@ -1,6 +1,6 @@
 FROM ekidd/rust-musl-builder:1.41.0 AS builder
 COPY . ./
-RUN cargo b --release --bin cddl
+RUN cargo b --release --bin cli
 
 FROM scratch
 COPY --from=builder /home/rust/src/target/x86_64-unknown-linux-musl/release/cddl /

cddl-lsp/.vscode/launch.json

@@ -21,7 +21,7 @@
       "port": 6009,
       "restart": true,
       "outFiles": ["${workspaceRoot}/server/out/**/*.js"],
-      "timeout": 20000
+      "timeout": 60000
     },
     {
       "name": "Language Server E2E Test",

cddl-lsp/client/package.json

@@ -3,7 +3,7 @@
   "description": "VSCode client for the Concise Data Definition Language (CDDL) language server implementation",
   "author": "Andrew Weiss <[email protected]>",
   "license": "MIT",
-  "version": "0.1.2",
+  "version": "0.1.3",
   "publisher": "vscode",
   "repository": {
     "type": "git",

cddl-lsp/package.json

@@ -4,7 +4,7 @@
   "description": "A language server extension for the Concise Data Definition Language (CDDL)",
   "author": "Andrew Weiss <[email protected]>",
   "license": "MIT",
-  "version": "0.1.2",
+  "version": "0.1.3",
   "repository": {
     "type": "git",
     "url": "https://github.com/anweiss/cddl"

cddl-lsp/server/package.json

@@ -1,7 +1,7 @@
 {
   "name": "cddl-lsp-server",
   "description": "Language server implementation for the Concise Data Definition Language (CDDL)",
-  "version": "0.1.2",
+  "version": "0.1.3",
   "author": "Andrew Weiss <[email protected]>",
   "license": "MIT",
   "engines": {

src/ast.rs

@@ -265,14 +265,14 @@ impl<'a> fmt::Display for Rule<'a> {
       } => {
         let mut rule_str = String::new();
 
-        rule_str.push_str(&format!("{}", rule));
+        rule_str.push_str(&rule.to_string());
 
         if let Some(comments) = comments_after_rule {
           if comments.any_non_newline() {
             if let Some(&"\n") = comments.0.first() {
-              rule_str.push_str(&format!("{}", comments.to_string()));
+              rule_str.push_str(&comments.to_string());
             } else {
-              rule_str.push_str(&format!(" {}", comments.to_string()));
+              rule_str.push_str(&format!(" {}", comments));
             }
           }
         }
@@ -286,12 +286,12 @@ impl<'a> fmt::Display for Rule<'a> {
       } => {
         let mut rule_str = String::new();
 
-        rule_str.push_str(&format!("{} ", rule.to_string()));
+        rule_str.push_str(&rule.to_string());
 
         if let Some(comments) = comments_after_rule {
           if comments.any_non_newline() {
             if let Some(&"\n") = comments.0.first() {
-              rule_str.push_str(&format!("{}", comments.to_string()));
+              rule_str.push_str(&comments.to_string());
             } else {
               rule_str.push_str(&format!(" {}", comments.to_string()));
             }
@@ -1098,8 +1098,19 @@ impl<'a> fmt::Display for Type2<'a> {
         if let Some(comments) = comments_before_group {
           if comments.any_non_newline() {
             non_newline_comments_before_group = true;
-            t2_str.push_str(&format!(" {}\t", comments));
-            t2_str.push_str(&group.to_string().trim_start().to_string());
+            for (idx, comment) in comments.0.iter().enumerate() {
+              if *comment != "\n" {
+                if idx == 0 {
+                  t2_str.push_str(&format!(" ;{}", comment));
+                } else {
+                  t2_str.push_str(&format!("\t;{}\n", comment));
+                }
+              } else {
+                t2_str.push_str("\n");
+              }
+            }
+
+            t2_str.push_str(&format!("\t{}", group.to_string().trim_start()));
           } else {
             t2_str.push_str(&group.to_string());
           }
@@ -1389,7 +1400,7 @@ impl<'a> GroupChoice<'a> {
     self
       .group_entries
       .iter()
-      .any(|ge| ge.0.has_trailing_comments())
+      .any(|ge| ge.0.has_trailing_comments() || ge.1.has_trailing_comments())
   }
 }
 
@@ -1403,7 +1414,7 @@ impl<'a> fmt::Display for GroupChoice<'a> {
         self.group_entries[0].0, self.group_entries[0].1
       ));
 
-      if self.group_entries[0].1.trailing_comments.is_none() {
+      if !self.group_entries[0].1.has_trailing_comments() {
         gc_str.push_str(" ");
       }
 
@@ -1446,14 +1457,23 @@ impl<'a> fmt::Display for GroupChoice<'a> {
       entries_with_comment_before_comma.push((idx, false));
     }
 
-    if self.group_entries.len() > 3 {
+    let has_trailing_comments_after_comma = self
+      .group_entries
+      .iter()
+      .any(|ge| ge.1.has_trailing_comments());
+
+    if self.group_entries.len() > 3
+      || (self.group_entries.len() <= 3 && has_trailing_comments_after_comma)
+    {
       gc_str.push_str("\n");
     } else {
       gc_str.push_str(" ");
     }
 
     for (idx, ge) in self.group_entries.iter().enumerate() {
-      if self.group_entries.len() > 3 {
+      if self.group_entries.len() > 3
+        || (self.group_entries.len() <= 3 && has_trailing_comments_after_comma)
+      {
         gc_str.push_str("\t");
       }
 
@@ -1469,25 +1489,31 @@ impl<'a> fmt::Display for GroupChoice<'a> {
         } else if idx != self.group_entries.len() - 1 {
           gc_str.push_str(&format!(", {}\n", ge.0.to_string().trim_end()));
         } else {
-          gc_str.push_str(&format!(", {}", ge.0));
+          gc_str.push_str(&format!(", {}", ge.0.to_string().trim_end()));
         }
       } else {
-        gc_str.push_str(&ge.0.to_string().trim_end().to_string());
+        gc_str.push_str(&format!(
+          "{}{}",
+          ge.0.to_string().trim_end(),
+          ge.1.to_string().trim_end()
+        ));
 
-        if idx != self.group_entries.len() - 1 {
-          gc_str.push_str(",");
-        }
+        // if idx != self.group_entries.len() - 1 {
+        //   gc_str.push_str(",");
+        // }
 
-        if self.group_entries.len() <= 3 {
+        if self.group_entries.len() <= 3 && !has_trailing_comments_after_comma {
           gc_str.push_str(" ");
         }
       }
 
-      if (self.group_entries.len() > 3 && entries_with_comment_before_comma.iter().all(|e| !e.1))
-        || (self.group_entries.len() > 3
-          && idx == self.group_entries.len() - 1
-          && !ge.0.has_trailing_comments())
-      {
+      if idx == self.group_entries.len() - 1 && self.group_entries.len() > 3 {
+        gc_str.push_str("\n");
+
+        break;
+      }
+
+      if self.group_entries.len() > 3 && entries_with_comment_before_comma.iter().all(|e| !e.1) {
         gc_str.push_str("\n");
       }
     }
@@ -1593,13 +1619,41 @@ impl<'a> fmt::Display for OptionalComma<'a> {
     }
 
     if let Some(comments) = &self.trailing_comments {
-      optcomma_str.push_str(&comments.to_string());
+      if comments.any_non_newline() {
+        if let Some(comment) = comments.0.first() {
+          if *comment != "\n" && self.optional_comma {
+            optcomma_str.push_str(" ");
+          }
+        }
+
+        for (idx, &comment) in comments.0.iter().enumerate() {
+          if idx == 0 && comment != "\n" {
+            optcomma_str.push_str(&format!(";{}\n", comment));
+          } else if idx == 0 {
+            optcomma_str.push_str(&comment.to_string());
+          } else if comment != "\n" {
+            optcomma_str.push_str(&format!("\t;{}\n", comment));
+          } else {
+            optcomma_str.push_str(&format!("\t{}", comment));
+          }
+        }
+      }
     }
 
     write!(f, "{}", optcomma_str)
   }
 }
 
+impl<'a> OptionalComma<'a> {
+  fn has_trailing_comments(&self) -> bool {
+    if let Some(comments) = &self.trailing_comments {
+      return comments.any_non_newline();
+    }
+
+    false
+  }
+}
+
 impl<'a> fmt::Display for GroupEntry<'a> {
   fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
     match self {
@@ -1673,7 +1727,14 @@ impl<'a> fmt::Display for GroupEntry<'a> {
             non_newline_comments_before_group = true;
 
             ge_str.push_str(&format!(" {}", comments));
-            ge_str.push_str(&format!("\t{}", group.to_string().trim_start()));
+
+            if !group
+              .group_choices
+              .iter()
+              .all(|gc| gc.group_entries.is_empty())
+            {
+              ge_str.push_str(&format!("\t{}", group.to_string().trim_start()));
+            }
           } else {
             ge_str.push_str(&group.to_string());
           }
@@ -2164,7 +2225,7 @@ mod tests {
         span: (0, 0, 0),
       }
       .to_string(),
-      " key1: \"value1\", key2: \"value2\" ".to_string()
+      " key1: \"value1\", key2: \"value2\", ".to_string()
     )
   }
 }

tests/fixtures/cddl/shelley.cddl

@@ -35,7 +35,7 @@ operational_cert = (
 	sigma: $signature
 )
 
-protocol_version = ( uint, uint ) 
+protocol_version = ( uint, uint )
 
 ; Do we want to use a Map here? Is it actually cheaper?
 ; Do we want to add extension points here?

tests/fixtures/cddl/socketplug.cddl

@@ -5,13 +5,13 @@ PersonalData = {
 	* $$personaldata-extensions
 }
 
-NameComponents = ( ? firstName: tstr, ? familyName: tstr ) 
+NameComponents = ( ? firstName: tstr, ? familyName: tstr )
 
 ; The above already works as is.
 ; But then, we can add later:
 
-$$personaldata-extensions //= ( favorite-salsa: tstr,) 
+$$personaldata-extensions //= ( favorite-salsa: tstr, )
 
 ; and again, somewhere else:
 
-$$personaldata-extensions //= ( shoesize: uint,)
+$$personaldata-extensions //= ( shoesize: uint, )