GitHub Issue #140 - better support for comments around :refer-clojure (…

…#141)

lib/standard-clojure-style.js

@@ -1498,7 +1498,6 @@
     const parenStack = []
     let insideNsForm = false
     let insideReferClojureForm = false
-    let referClojureLineNo = -1
     let insideRequireForm = false
     let requireFormLineNo = -1
     let insideImportForm = false
@@ -1565,6 +1564,7 @@
     let idOfLastNodeInsideReaderComment = -1
     let requireRenameIdx = -1
     let skipNodesUntilWeReachThisId = -1
+    let sectionToAttachEolCommentsTo = null
 
     while (continueParsingNsForm) {
       const node = nodesArr[idx]
@@ -1576,30 +1576,34 @@
         nsNodeIdx = idx
       } else if (insideNsForm && isReferClojureNode(node)) {
         insideReferClojureForm = true
-        referClojureLineNo = lineNo
+        sectionToAttachEolCommentsTo = 'refer-clojure'
         referClojureNodeIdx = idx
         beyondNsMetadata = true
       } else if (insideNsForm && isRequireNode(node)) {
         insideRequireForm = true
         requireFormLineNo = lineNo
         requireNodeIdx = idx
         beyondNsMetadata = true
+        sectionToAttachEolCommentsTo = 'require'
       } else if (insideNsForm && isImportNode(node)) {
         insideImportForm = true
         importFormLineNo = lineNo
         importNodeIdx = idx
         importNodeParenNestingDepth = parenNestingDepth
         beyondNsMetadata = true
+        sectionToAttachEolCommentsTo = 'import'
       } else if (insideNsForm && isRequireMacrosKeyword(node)) {
         insideRequireMacrosForm = true
         requireMacrosNodeIdx = idx
         requireMacrosLineNo = lineNo
         requireMacrosParenNestingDepth = parenNestingDepth
         beyondNsMetadata = true
+        sectionToAttachEolCommentsTo = 'require-macros'
       } else if (insideNsForm && isGenClassNode(node)) {
         insideGenClass = true
         genClassNodeIdx = idx
         beyondNsMetadata = true
+        sectionToAttachEolCommentsTo = 'gen-class'
       }
 
       if (isParenOpener(node)) {
@@ -1815,7 +1819,7 @@
         } else if (requireFormLineNo === lineNo && activeRequireIdx >= 0) {
           result.requires[activeRequireIdx].commentAfter = commentAtEndOfLine
           lineOfLastCommentRecording = lineNo
-        } else if (lineNo === referClojureLineNo && result.referClojure) {
+        } else if (sectionToAttachEolCommentsTo === 'refer-clojure' && result.referClojure) {
           result.referClojureCommentAfter = commentAtEndOfLine
           lineOfLastCommentRecording = lineNo
         } else if (importFormLineNo === lineNo && !result.importsObj) {
@@ -2593,31 +2597,40 @@
     return s
   }
 
-  function formatReferClojureSingleKeyword (kwd, symbolsArr) {
+  function formatReferClojureSingleKeyword (ns, excludeOrOnly) {
+    const symbolsArr = ns.referClojure[excludeOrOnly]
+    const kwd = strConcat(':', excludeOrOnly)
+
     const platforms = getPlatformsFromArray(symbolsArr)
     const numPlatforms = arraySize(platforms)
     const symbolsForAllPlatforms = arrayPluck(filterOnPlatform(symbolsArr, false), 'symbol')
     const numSymbolsForAllPlatforms = arraySize(symbolsForAllPlatforms)
 
     // there are no reader conditionals: print all of the symbols
     if (numPlatforms === 0) {
-      let s = '\n  (:refer-clojure '
+      let s = '\n'
+      s = printCommentsAbove(s, ns.referClojureCommentsAbove, '  ')
+      s = strConcat(s, '  (:refer-clojure ')
       s = strConcat(s, formatKeywordFollowedByListOfSymbols(kwd, symbolsForAllPlatforms))
       s = strConcat(s, ')')
       return s
 
     // all symbols are for a single platform: wrap the entire (:refer-clojure) in a single reader conditional
     } else if (numPlatforms === 1 && numSymbolsForAllPlatforms === 0) {
       const symbols2 = arrayPluck(symbolsArr, 'symbol')
+
       let s = strConcat3('\n  #?(', platforms[0], '\n')
+      s = printCommentsAbove(s, ns.referClojureCommentsAbove, '     ')
       s = strConcat(s, '     (:refer-clojure ')
       s = strConcat(s, formatKeywordFollowedByListOfSymbols(kwd, symbols2))
       s = strConcat(s, '))')
       return s
 
     // all symbols are for specific platforms, ie: every symbol is wrapped in a reader conditional
     } else if (numPlatforms > 1 && numSymbolsForAllPlatforms === 0) {
-      let s = '\n  (:refer-clojure\n'
+      let s = '\n'
+      s = printCommentsAbove(s, ns.referClojureCommentsAbove, '  ')
+      s = strConcat(s, '  (:refer-clojure\n')
       s = strConcat3(s, '    ', kwd)
       s = strConcat(s, ' #?@(')
 
@@ -2643,13 +2656,13 @@
 
       s = strConcat(s, '))')
 
-      // FIXME: add commentAfter here?
-
       return s
 
       // we have a mix of symbols for all platforms and some for specific platforms
     } else {
-      let s = '\n  (:refer-clojure\n'
+      let s = '\n'
+      s = printCommentsAbove(s, ns.referClojureCommentsAbove, '  ')
+      s = strConcat(s, '  (:refer-clojure\n')
       s = strConcat3(s, '    ', kwd)
       s = strConcat(s, ' [')
       s = strConcat(s, strJoin(symbolsForAllPlatforms, ' '))
@@ -2683,44 +2696,45 @@
 
       s = strConcat(s, ')])')
 
-      // FIXME: add commentAfter here?
-
       return s
     }
   }
 
-  function formatReferClojure (referClojure) {
-    const keys = getReferClojureKeys(referClojure)
+  function formatReferClojure (ns) {
+    const keys = getReferClojureKeys(ns.referClojure)
     const numKeys = arraySize(keys)
 
     // there are no :refer-clojure items, we are done
     if (numKeys === 0) {
       return ''
     // there is only :exclude
     } else if (numKeys === 1 && keys[0] === ':exclude') {
-      return formatReferClojureSingleKeyword(':exclude', referClojure.exclude)
+      return formatReferClojureSingleKeyword(ns, 'exclude')
 
     // there is only :only
     } else if (numKeys === 1 && keys[0] === ':only') {
-      return formatReferClojureSingleKeyword(':only', referClojure.only)
+      return formatReferClojureSingleKeyword(ns, 'only')
 
     // there is only :rename
     } else if (numKeys === 1 && keys[0] === ':rename') {
-      const platforms = getPlatformsFromArray(referClojure.rename)
+      const platforms = getPlatformsFromArray(ns.referClojure.rename)
       const numPlatforms = arraySize(platforms)
-      const nonPlatformSpecificRenames = filterOnPlatform(referClojure.rename, false)
+      const nonPlatformSpecificRenames = filterOnPlatform(ns.referClojure.rename, false)
       const numNonPlatformSpecificRenames = arraySize(nonPlatformSpecificRenames)
       const allRenamesForSamePlatform = numNonPlatformSpecificRenames === 0 && arraySize(platforms) > 0
 
       if (numPlatforms === 0) {
-        let s = '\n  (:refer-clojure :rename {'
-        s = strConcat(s, formatRenamesList(referClojure.rename))
+        let s = '\n'
+        s = printCommentsAbove(s, ns.referClojureCommentsAbove, '  ')
+        s = strConcat(s, '  (:refer-clojure :rename {')
+        s = strConcat(s, formatRenamesList(ns.referClojure.rename))
         s = strConcat(s, '})')
         return s
       } else if (numPlatforms === 1 && allRenamesForSamePlatform) {
         let s = strConcat3('\n  #?(', platforms[0], '\n')
+        s = printCommentsAbove(s, ns.referClojureCommentsAbove, '     ')
         s = strConcat(s, '     (:refer-clojure :rename {')
-        s = strConcat(s, formatRenamesList(referClojure.rename))
+        s = strConcat(s, formatRenamesList(ns.referClojure.rename))
         s = strConcat(s, '}))')
         return s
       } else {
@@ -2731,7 +2745,7 @@
         let platformIdx = 0
         while (platformIdx < numPlatforms) {
           const platformStr = platforms[platformIdx]
-          const platformRenames = filterOnPlatform(referClojure.rename, platformStr)
+          const platformRenames = filterOnPlatform(ns.referClojure.rename, platformStr)
 
           if (platformIdx === 0) {
             s = strConcat3(s, platformStr, ' [')
@@ -2754,21 +2768,21 @@
     } else {
       let s = '\n  (:refer-clojure'
 
-      if (referClojure.exclude && arraySize(referClojure.exclude) > 0) {
-        const excludeSymbols = arrayPluck(referClojure.exclude, 'symbol')
+      if (ns.referClojure.exclude && arraySize(ns.referClojure.exclude) > 0) {
+        const excludeSymbols = arrayPluck(ns.referClojure.exclude, 'symbol')
         s = strConcat(s, '\n    ')
         s = strConcat(s, formatKeywordFollowedByListOfSymbols(':exclude', excludeSymbols))
       }
 
-      if (referClojure.only && arraySize(referClojure.only) > 0) {
-        const onlySymbols = arrayPluck(referClojure.only, 'symbol')
+      if (ns.referClojure.only && arraySize(ns.referClojure.only) > 0) {
+        const onlySymbols = arrayPluck(ns.referClojure.only, 'symbol')
         s = strConcat(s, '\n    ')
         s = strConcat(s, formatKeywordFollowedByListOfSymbols(':only', onlySymbols))
       }
 
-      if (referClojure.rename && arraySize(referClojure.rename) > 0) {
+      if (ns.referClojure.rename && arraySize(ns.referClojure.rename) > 0) {
         s = strConcat(s, '\n    :rename {')
-        s = strConcat(s, formatRenamesList(referClojure.rename))
+        s = strConcat(s, formatRenamesList(ns.referClojure.rename))
         s = strConcat(s, '}')
       }
 
@@ -2798,10 +2812,12 @@
       numImports = arraySize(ns.imports)
     }
 
+    let commentOutsideNsForm2 = null
     const hasGenClass = !!ns.genClass
     const importsIsLastMainForm = numImports > 0 && !hasGenClass
     const requireIsLastMainForm = numRequires > 0 && !importsIsLastMainForm && !hasGenClass
     const requireMacrosIsLastMainForm = numRequireMacros > 0 && numRequires === 0 && numImports === 0 && !hasGenClass
+    const referClojureIsLastMainForm = ns.referClojure && numRequireMacros === 0 && numRequires === 0 && numImports === 0 && !hasGenClass
     let trailingParensArePrinted = false
 
     if (isString(ns.docstring)) {
@@ -2833,9 +2849,16 @@
     }
 
     // FIXME - we need reader conditionals for :refer-clojure here
-    // FIXME - comments for :refer-clojure
     if (ns.referClojure) {
-      outTxt = strConcat(outTxt, formatReferClojure(ns.referClojure))
+      outTxt = strConcat(outTxt, formatReferClojure(ns))
+
+      if (isNonBlankString(ns.referClojureCommentAfter)) {
+        if (referClojureIsLastMainForm) {
+          commentOutsideNsForm2 = ns.referClojureCommentAfter
+        } else {
+          outTxt = strConcat3(outTxt, ' ', ns.referClojureCommentAfter)
+        }
+      }
     }
 
     if (numRequireMacros > 0) {
@@ -3291,6 +3314,10 @@
       outTxt = strConcat(outTxt, ')')
     }
 
+    if (isNonBlankString(commentOutsideNsForm2)) {
+      outTxt = strConcat3(outTxt, ' ', commentOutsideNsForm2)
+    }
+
     return outTxt
   }
 

test_format/ns.eno

@@ -25,6 +25,7 @@
 
 --Input
 (ns com.example.my-application
+  ;; aaa
   (refer-clojure :exclude [get])
   (require
    [clojure.string :as string])
@@ -38,6 +39,7 @@
 
 --Expected
 (ns com.example.my-application
+  ;; aaa
   (:refer-clojure :exclude [get])
   (:require
    [clojure.string :as string])
@@ -224,19 +226,21 @@
 
 # refer-clojure :rename with reader conditional
 
-> FIXME: need to check this is valid reader conditional syntax
-
 --Input
 (ns com.example.my-app
+  ;; comment 1
+  ;; comment 2
   (:refer-clojure
-    :rename {#?(:cljs [print core-print get g])})
+    :rename {#?(:cljs [print core-print get g])}) ;; comment 3
   )
 --Input
 
 --Expected
 (ns com.example.my-app
   #?(:cljs
-     (:refer-clojure :rename {get g, print core-print})))
+     ;; comment 1
+     ;; comment 2
+     (:refer-clojure :rename {get g, print core-print}))) ;; comment 3
 --Expected
 
 # how to ns: require, as, refer
@@ -1727,3 +1731,98 @@
 
 (defprotocol ToJson (-to-json [value buffer]))
 --Expected
+
+# GitHub Issue #140 - :refer-clojure comments 1
+
+--Input
+(ns com.example.my-app
+  ;; aaa
+  ;; bbb
+  (:refer-clojure :only [number? -> set]) ; ccc
+  )
+--Input
+
+--Expected
+(ns com.example.my-app
+  ;; aaa
+  ;; bbb
+  (:refer-clojure :only [-> number? set])) ; ccc
+--Expected
+
+# GitHub Issue #140 - :refer-clojure comments 2
+
+--Input
+(ns com.example.my-app
+  ;; aaa
+  ;; bbb
+  (:refer-clojure :only [number? -> set ; ccc
+  ]))
+--Input
+
+--Expected
+(ns com.example.my-app
+  ;; aaa
+  ;; bbb
+  (:refer-clojure :only [-> number? set])) ; ccc
+--Expected
+
+# GitHub Issue #140 - :refer-clojure comments 3
+
+--Input
+(ns com.example.my-app
+  (:require [aaa])
+  ;; aaa
+  ;; bbb
+  (:refer-clojure :only [number? -> set]) ; ccc
+  )
+--Input
+
+--Expected
+(ns com.example.my-app
+  ;; aaa
+  ;; bbb
+  (:refer-clojure :only [-> number? set]) ; ccc
+  (:require
+   [aaa]))
+--Expected
+
+# GitHub Issue #140 - :refer-clojure comments 4
+
+--Input
+(ns com.example.my-app
+  (:require [aaa])
+  ;; aaa
+  ;; bbb
+  #?(:clj
+     (:refer-clojure :only [number? -> set])) ;; ccc
+  )
+--Input
+
+--Expected
+(ns com.example.my-app
+  #?(:clj
+     ;; aaa
+     ;; bbb
+     (:refer-clojure :only [-> number? set])) ;; ccc
+  (:require
+   [aaa]))
+--Expected
+
+# GitHub Issue #140 - :refer-clojure comments 5
+
+--Input
+(ns com.example.my-app
+  ;; aaa
+  ;; bbb
+  #?(:clj
+     (:refer-clojure :only [number? -> set])) ;; ccc
+  )
+--Input
+
+--Expected
+(ns com.example.my-app
+  #?(:clj
+     ;; aaa
+     ;; bbb
+     (:refer-clojure :only [-> number? set]))) ;; ccc
+--Expected