Comments running closer to prettier for edge cases

Author: Janther

src/slang-comments/handlers/handle-contract-definition-comments.ts

@@ -1,7 +1,6 @@
 import { NonterminalKind } from '@nomicfoundation/slang/kinds/index.js';
 import { util } from 'prettier';
 import { getNextNonSpaceNonCommentCharacter } from '../../slang-utils/backward-compatibility.js';
-import addCollectionNodeFirstComment from './add-collection-node-first-comment.js';
 import addCollectionNodeLastComment from './add-collection-node-last-comment.js';
 
 import type { HandlerParams } from './types';
@@ -52,13 +51,6 @@ export default function handleContractDefinitionComments({
       );
       return true;
     }
-
-    // If there's no InheritanceSpecifier, the comment before the body is
-    // assumed to be intended at the beginning of the body.
-    if (followingNode?.kind === NonterminalKind.ContractMembers) {
-      addCollectionNodeFirstComment(followingNode, comment);
-      return true;
-    }
   }
 
   return false;

src/slang-comments/handlers/handle-modifier-invocation-comments.ts

@@ -0,0 +1,40 @@
+import { NonterminalKind } from '@nomicfoundation/slang/kinds/index.js';
+import { util } from 'prettier';
+import { getNextNonSpaceNonCommentCharacter } from '../../slang-utils/backward-compatibility.js';
+import addCollectionNodeFirstComment from './add-collection-node-first-comment.js';
+
+import type { HandlerParams } from './types';
+
+const { addTrailingComment } = util;
+
+export default function handleModifierInvocationComments({
+  text,
+  precedingNode,
+  enclosingNode,
+  followingNode,
+  comment
+}: HandlerParams): boolean {
+  if (enclosingNode?.kind !== NonterminalKind.ModifierInvocation) {
+    return false;
+  }
+
+  const nextCharacter = getNextNonSpaceNonCommentCharacter(text, comment);
+
+  // The last comments before the body.
+  if (
+    precedingNode?.kind === NonterminalKind.IdentifierPath &&
+    nextCharacter === '(' &&
+    followingNode?.kind === NonterminalKind.ArgumentsDeclaration &&
+    followingNode.variant.kind ===
+      NonterminalKind.PositionalArgumentsDeclaration
+  ) {
+    if (followingNode.variant.arguments.items.length === 0) {
+      addTrailingComment(enclosingNode, comment);
+    } else {
+      addCollectionNodeFirstComment(followingNode.variant.arguments, comment);
+    }
+    return true;
+  }
+
+  return false;
+}

src/slang-comments/handlers/index.ts

@@ -4,6 +4,7 @@ import handleElseBranchComments from './handle-else-branch-comments.js';
 import handleIfStatementComments from './handle-if-statement-comments.js';
 import handleInterfaceDefinitionComments from './handle-interface-definition-comments.js';
 import handleLibraryDefinitionComments from './handle-library-definition-comments.js';
+import handleModifierInvocationComments from './handle-modifier-invocation-comments.js';
 import handleParametersDeclarationComments from './handle-parameters-declaration-comments.js';
 import handlePositionalArgumentsDeclarationComments from './handle-positional-arguments-declaration-comments.js';
 import handleWhileStatementComments from './handle-while-statement-comments.js';
@@ -16,6 +17,7 @@ export default [
   handleIfStatementComments,
   handleInterfaceDefinitionComments,
   handleLibraryDefinitionComments,
+  handleModifierInvocationComments,
   handleParametersDeclarationComments,
   handlePositionalArgumentsDeclarationComments,
   handleWhileStatementComments,

src/slang-nodes/ModifierInvocation.ts

@@ -1,5 +1,5 @@
 import { NonterminalKind } from '@nomicfoundation/slang/kinds/index.js';
-import { isComment } from '../slang-utils/is-comment.js';
+import { isBlockComment } from '../slang-utils/is-comment.js';
 import { getNodeMetadata, updateMetadata } from '../slang-utils/metadata.js';
 import { IdentifierPath } from './IdentifierPath.js';
 import { ArgumentsDeclaration } from './ArgumentsDeclaration.js';
@@ -50,7 +50,9 @@ export class ModifierInvocation implements SlangNode {
         this.arguments.variant.kind ===
           NonterminalKind.PositionalArgumentsDeclaration &&
         this.arguments.variant.arguments.items.length === 0 && // no arguments
-        !ast.arguments!.variant.cst.children().some((child) => isComment(child)) // no comments, at this point we need to check the CST
+        !ast
+          .arguments!.variant.cst.children()
+          .some((child) => isBlockComment(child)) // no comments, at this point we need to check the CST
       ) {
         delete this.arguments;
       }

src/slang-nodes/SourceUnit.ts

@@ -31,7 +31,9 @@ export class SourceUnit implements SlangNode {
 
     metadata = updateMetadata(metadata, [this.members]);
 
-    this.comments = metadata.comments;
+    // Because of comments being extracted like a russian doll, the order needs
+    // to be fixed at the end.
+    this.comments = metadata.comments.sort((a, b) => a.loc.start - b.loc.start);
     this.loc = metadata.loc;
   }
 

src/slang-utils/is-comment.ts

@@ -7,7 +7,7 @@ import type { AstNode, BlockComment, Comment } from '../slang-nodes';
 export const isBlockComment = createKindCheckFunction([
   TerminalKind.MultiLineComment,
   TerminalKind.MultiLineNatSpecComment
-]) as (node: AstNode) => node is BlockComment;
+]) as (node: AstNode | Node) => node is BlockComment;
 
 export const isComment = createKindCheckFunction([
   TerminalKind.MultiLineComment,

tests/config/run-format-test.js

@@ -40,17 +40,6 @@ const unstableAstTests = new Map(
   }),
 );
 
-const testsWithSlang = new Map(
-  [
-    // "Comments/Comments.sol", // TODO: finish Comments
-  ].map((fixture) => {
-    const [file, testSlang = () => true] = Array.isArray(fixture)
-      ? fixture
-      : [fixture];
-    return [path.join(__dirname, "../format/", file), testSlang];
-  }),
-);
-
 const testsWithAstChanges = new Map(
   [
     "Parentheses/AddNoParentheses.sol",
@@ -95,16 +84,6 @@ const isAstUnstable = (filename, options) => {
   return testFunction(options);
 };
 
-const shouldTestSlang = (filename, options) => {
-  const testFunction = testsWithSlang.get(filename);
-
-  if (!testFunction) {
-    return false;
-  }
-
-  return testFunction(options);
-};
-
 const shouldCompareBytecode = (filename, options) => {
   const testFunction = testsWithAstChanges.get(filename);
 
@@ -309,22 +288,6 @@ async function runTest({
     }),
   ).toMatchSnapshot();
 
-  if (shouldTestSlang(filename, formatOptions)) {
-    const { input, output } = formatResult;
-    const slangOptions = {
-      ...formatOptions,
-      parser: "slang",
-    };
-    console.log(filename);
-    const prettier = await getPrettier();
-    const slangOutput = await prettier.format(input, slangOptions);
-
-    // const slangOutput2 = await prettier.format(output, slangOptions);
-
-    expect(slangOutput).toEqual(output);
-    // expect(slangOutput2).toEqual(output);
-  }
-
   if (!FULL_TEST) {
     return;
   }

tests/format/Comments/__snapshots__/format.test.js.snap

@@ -3,7 +3,7 @@
 exports[`Comments.sol - {"compiler":"0.4.24"} format 1`] = `
 ====================================options=====================================
 compiler: "0.4.24"
-parsers: ["solidity-parse"]
+parsers: ["slang-solidity"]
 printWidth: 80
                                                                                 | printWidth
 =====================================input======================================
@@ -207,8 +207,7 @@ contract Comments3 is
 {
     // solhint-disable-previous-line no-empty-blocks
 
-    function someFunction() {} /*1*/
-    /*2
+    function someFunction() {} /*1*/ /*2
      */
 }
 
@@ -223,7 +222,7 @@ contract Comments4 is
     // solhint-disable-previous-line no-empty-blocks
 }
 
-/*nice name*/ contract Comments5 {
+contract Comments5 /*nice name*/ {
     // solhint-disable-previous-line no-empty-blocks
 }
 
@@ -289,7 +288,7 @@ interface Comments10 {
         // the first value
         // the second value
         // the lats value
-    )/* comment outside the parameters */ external;
+    ) /* comment outside the parameters */ external;
 
     function someOtherFunction(/* checking for Block comment */) external;
 }
@@ -319,14 +318,12 @@ contract Comments13 {
     function commentInModifierInvocation()
         external
         // comment 1
-        // comment 2
-        // comment 3
-        modifier1 // comment 4
+        modifier1 // comment 2
+        // comment 3 // comment 4
         // comment 5
-        // comment 6
-        modifier2(/* comment 7 */) // comment 8
-        // comment 9
+        modifier2(/* comment 7 */) // comment 6 // comment 8
         modifier3(
+            // comment 9
             // comment 10
             param1 // comment 11
             // comment 12

tests/format/Comments/format.test.js

@@ -1 +1 @@
-runFormatTest(import.meta, ['solidity-parse'], { compiler: '0.4.24' });
+runFormatTest(import.meta, ['slang-solidity'], { compiler: '0.4.24' });