prettier-plugin-glsl - npm Package Compare versions

Comparing version 0.0.1 to 0.0.2

lib/errors.d.ts

@@ -1,2 +0,2 @@
-export declare const ERRORS: Record<string, string>;
-//# sourceMappingURL=errors.d.ts.map
+export declare const ERRORS: Record<string, string>
+//# sourceMappingURL=errors.d.ts.map

package.json

@@ -12,4 +12,4 @@ {
   ],
-  "version": "0.0.1",
-  "description": "Prettier (https://prettier.io) plugin for GLSL (Open GL Shading Language).",
+  "version": "0.0.2",
+  "description": "Prettier (https://prettier.io) plugin for GLSL (OpenGL Shading Language).",
   "exports": "./lib/prettier-plugin.cjs.js",
@@ -26,6 +26,10 @@ "type": "commonjs",
     "build": "rollup --config",
-    "clean": "rimraf lib",
-    "fmt": "prettier --write ."
+    "clean": "rimraf lib"
   },
-  "keywords": [],
+  "keywords": [
+    "formatter",
+    "glsl",
+    "prettier",
+    "plugin"
+  ],
   "author": "Adrian Leonhard",
@@ -35,3 +39,2 @@ "license": "MIT",
     "@netflix/nerror": "^1.1.3",
-    "@types/dedent": "^0.7.0",
     "chevrotain": "^10.1.2",
@@ -46,10 +49,11 @@ "colors": "^1.4.0",
     "@rollup/plugin-typescript": "^8.3.1",
+    "@types/dedent": "^0.7.0",
     "@types/jest": "^27.4.1",
-    "@types/lodash": "^4.14.179",
+    "@types/lodash": "^4.14.180",
     "@types/node": "^17.0.21",
     "@types/node-fetch": "^2.6.1",
     "@types/prettier": "^2.4.4",
-    "@typescript-eslint/eslint-plugin": "^5.14.0",
-    "@typescript-eslint/parser": "^5.14.0",
-    "eslint": "^8.10.0",
+    "@typescript-eslint/eslint-plugin": "^5.15.0",
+    "@typescript-eslint/parser": "^5.15.0",
+    "eslint": "^8.11.0",
     "eslint-config-prettier": "^8.5.0",
@@ -61,3 +65,3 @@ "eslint-plugin-cflint": "^1.0.0",
     "node-fetch": "^2.6.7",
-    "rollup": "^2.70.0",
+    "rollup": "^2.70.1",
     "rollup-plugin-terser": "^7.0.2",
@@ -64,0 +68,0 @@ "ts-jest": "^27.1.3",

README.md

@@ -0,3 +1,43 @@
+![npm](https://img.shields.io/npm/v/prettier-plugin-glsl?style=flat-square)
+![NPM](https://img.shields.io/npm/l/prettier-plugin-glsl?style=flat-square)
+
 # prettier-plugin-glsl
 
+This is a plugin for [Prettier](https://prettier.io), the opinionated code
+formatter, for GLSL, the shading language used in WebGL and other places. It
+uses a custom parser based on [chevrotain](https://chevrotain.io/) and does not
+require any external dependencies.
+
+**NB: this is still in active development, breaking/formatting changes may be
+included in any version.**
+
+## Formatting
+
+This plugin tries to match the formatting rules for JavaScript as closely as
+possible. Issues or PRs which make the formatting closer to JavaScript are
+welcome.
+
+### Formatting of macros
+
+This plugin will attempt to parse `#define` macros as top-level declarations,
+statements or expressions. If successful, these will be formatted as usual.
+
+For example,
+`#define MAX3(genType) genType max3(genType a, genType b, genType c) { /* comment */ return max(max(a, b), c); }`
+will be formatted as.
+
+```glsl
+#define MAX3(genType)                                                          \
+  genType max3(genType a, genType b, genType c) { \
+    /* comment */                                                              \
+ return max(max(a, b), c);                                                  \
+ }
+```
+
+### Formatting of comments
+
+Comments which start with `/**` will be passed to the markdown formatter.
+
+For an example, see [./builtins.glsl](./builtins.glsl).
+
 ## Installation
@@ -8,1 +48,74 @@
 ```
+
+Prettier will automatically pick up the plugin. The following extensions will be
+recognized as GLSL files by default.
+
+`.fp` `.frag` `.frg` `.fs` `.fsh` `.fshader` `.geo` `.geom` `.glsl` `.glslf`
+`.glslv` `.gs` `.gshader` `.rchit` `.rmiss` `.shader` `.tesc` `.tese` `.vert`
+`.vrx` `.vsh` `.vshader`
+
+## Limitations due to preprocessor
+
+As GLSL includes a C++-style preprocessor, this presents some difficulties when
+formatting. For example, the plugin does not attempt to be able to format crazy
+(but technically valid) constructs such as:
+
+```glsl
+#define LPAREN
+void main() LPAREN
+}
+```
+
+Instead, the plugin effectively treats preprocessor directives such as `#define`
+as their own statements. This covers most cases, for example, the following
+works fine:
+
+```glsl
+#define AA 2
+#define ZERO (min(iFrame, 0))
+void main() {
+    // ...
+    #if AA > 1
+    for (int m = ZERO; m < AA; m++)
+    for (int n = ZERO; n < AA; n++) {
+        // pixel coordinates
+        vec2 o = vec2(float(m), float(n)) / float(AA) - 0.5;
+        vec2 p = (2.0 * (fragCoord + o) - iResolution.xy) / iResolution.y;
+        #else
+        vec2 p = (2.0 * fragCoord - iResolution.xy) / iResolution.y;
+        #endif
+
+        // use p
+
+        #if AA > 1
+    }
+    tot /= float(AA * AA);
+    #endif
+}
+```
+
+However, the following does not, as the `#else` and `#endif` are treated as the
+bodies of the if-statement, which leads to the `else` following a `{}` block
+instead of an if block, which is invalid.
+
+```glsl
+#if FOO
+if (a())
+#else
+if (b())
+#endif
+{ }
+else
+{ }
+```
+
+In general this approach works well. Of the top 100
+[Shadertoy](https://www.shadertoy.com/) shaders, 145/152 compilation units (95%)
+can be formatted without any changes.
+
+- 1 has actually invalid code. (It doesn't cause compilation errors as it is
+  excluded via `#if/#endif`.)
+- 1 uses a not self-contained function macro. (It requires a specific token
+  before its call.)
+- 4 mix `#if/#else/#endif` and `if/else` which leads to issues as above.
+- 1 uses complicated macro constructions to compose music.

src/checker.ts

@@ -48,3 +48,9 @@ /* eslint-disable @typescript-eslint/restrict-template-expressions */
 import { parseInput } from "./parser"
-import { allDefined, assertNever, CheckError, mapExpandedLocation, safeMap } from "./util"
+import {
+  allDefined,
+  assertNever,
+  CheckError,
+  mapExpandedLocation,
+  safeMap,
+} from "./util"
 import { ERRORS } from "./errors"
@@ -51,0 +57,0 @@

src/nodes.ts

@@ -5,4 +5,4 @@ import { IToken } from "chevrotain"
   macroSource?: Token
-  // original line number, as parsed from input with applied line continuations
-  // needed by preprocessor to figure out which tokens are in one line
+  // Line number, as parsed from input with applied line continuations.
+  // Needed by preprocessor to figure out which tokens are in one line.
   lineNoCont?: number
@@ -9,0 +9,0 @@ }

src/parser.ts

@@ -56,5 +56,11 @@ /* eslint-disable @typescript-eslint/member-ordering */
 } from "./nodes"
-import { ALL_TOKENS, checkLexingErrors, GLSL_LEXER, RESERVED_KEYWORDS, TOKEN } from "./lexer"
+import {
+  ALL_TOKENS,
+  checkLexingErrors,
+  GLSL_LEXER,
+  RESERVED_KEYWORDS,
+  TOKEN,
+} from "./lexer"
 import { DEV, ExpandedLocation, substrContext } from "./util"
-import { applyLineContinuations } from "./preprocessor"
+import { applyLineContinuations, fixLocations } from "./preprocessor"
 
@@ -1697,7 +1703,9 @@ // eslint-disable-next-line @typescript-eslint/no-unused-vars
 export function parseInput(originalInput: string): TranslationUnit {
-  const { result: input, changes } = applyLineContinuations(originalInput)
+  const noLineCont = applyLineContinuations(originalInput)
 
-  const lexingResult = GLSL_LEXER.tokenize(input)
-  checkLexingErrors(input, lexingResult)
+  const lexingResult = GLSL_LEXER.tokenize(noLineCont.result)
+  checkLexingErrors(noLineCont.result, lexingResult)
 
+  fixLocations(lexingResult.tokens, noLineCont.changes)
+
   checkTokenErrors(lexingResult)
@@ -1708,3 +1716,3 @@
   const result = GLSL_PARSER.translationUnit()
-  checkParsingErrors(input, GLSL_PARSER.errors)
+  checkParsingErrors(noLineCont.result, GLSL_PARSER.errors)
 
@@ -1711,0 +1719,0 @@ const ppDefs = GLSL_PARSER.ppDefs

src/prettier-plugin.ts

@@ -18,4 +18,6 @@ // noinspection JSUnusedGlobalSymbols
   AbstractVisitor,
+  AssignmentExpression,
   BinaryExpression,
   CommaExpression,
+  Declarator,
   isExpression,
@@ -55,2 +57,3 @@ isToken,
     fill,
+    lineSuffixBoundary,
   },
@@ -111,2 +114,6 @@ } = doc
 
+function locStart(node: Node | IToken) {
+  return (isToken(node) ? node : node.firstToken!).startOffset
+}
+
 function locEnd(node: Node | IToken) {
@@ -126,5 +133,3 @@ return (isToken(node) ? node : node.lastToken!).endOffset! + 1
     astFormat: "glsl-ast",
-    locStart(node: Node | IToken) {
-      return (isToken(node) ? node : node.firstToken!).startOffset
-    },
+    locStart,
     locEnd,
@@ -349,2 +354,81 @@ },
 
+function isAssignment(node: Node): node is AssignmentExpression {
+  return node.kind === "assignmentExpression"
+}
+
+function isAssignmentOrVariableDeclarator(
+  node: Node,
+): node is AssignmentExpression | Declarator {
+  return node.kind === "assignmentExpression" || node.kind === "declarator"
+}
+
+function hasLeadingOwnLineComment(
+  originalText: string,
+  rightNode: Node,
+): boolean {
+  // TODO
+  return false
+}
+
+function chooseAssignmentLayout(
+  path: AstPath<Node | IToken>,
+  options: ParserOptions<Node | IToken> & { inMacro?: Token[] },
+  rightPropertyName: string,
+):
+  | "break-after-operator"
+  | "chain"
+  | "chain-tail"
+  | "only-left"
+  | "never-break-after-operator"
+  | "fluid" {
+  const node = path.getValue() as Node
+  const rightNode = (node as any)[rightPropertyName] as Node
+
+  if (!rightNode) {
+    return "only-left"
+  }
+
+  // Short assignment chains (only 2 segments) are NOT formatted as chains.
+  //   1) a = b = c; (expression statements)
+  //   2) int a = b = c;
+
+  const isTail = !isAssignment(rightNode)
+  const shouldUseChainFormatting = path.match(
+    isAssignment,
+    isAssignmentOrVariableDeclarator,
+    (node: Node) =>
+      !isTail ||
+      (node.kind !== "expressionStatement" &&
+        node.kind !== "initDeclaratorListDeclaration"),
+  )
+
+  if (shouldUseChainFormatting) {
+    return !isTail ? "chain" : "chain-tail"
+  }
+
+  const isHeadOfLongChain = !isTail && isAssignment(rightNode.rhs)
+
+  if (
+    isHeadOfLongChain ||
+    hasLeadingOwnLineComment(options.originalText, rightNode)
+  ) {
+    return "break-after-operator"
+  }
+
+  if (
+    rightNode.kind === "binaryExpression" ||
+    rightNode.kind === "commaExpression" ||
+    (rightNode.kind === "conditionalExpression" &&
+      rightNode.condition.kind === "binaryExpression")
+  ) {
+    return "break-after-operator"
+  }
+
+  if (rightNode.kind === "constantExpression") {
+    return "never-break-after-operator"
+  }
+
+  return "fluid"
+}
+
 export const printers: Plugin<Node | IToken>["printers"] = {
@@ -496,9 +580,27 @@ "glsl-ast": {
           case "initDeclaratorListDeclaration":
-          case "structDeclaration":
+          case "structDeclaration": {
+            const printed = path.map(print, "declarators")
+
+            const parentNode = path.getParentNode() as Node
+            const isParentForLoop = parentNode.kind === "forStatement"
+            const hasInit = n.declarators.some((decl) => decl.init)
+
+            // If we have multiple declarations, or if the only declaration is
+            // on its own line due to a comment, indent the declarations.
+            // TODO: hasComment?
             return group([
               p<typeof n>("fsType"),
               " ",
-              indent(join([",", line], path.map(print, "declarators"))),
+              printed.length === 1
+                ? printed
+                : indent(
+                    join(
+                      [",", hasInit && !isParentForLoop ? hardline : line],
+                      printed,
+                    ),
+                  ),
               ";",
             ])
+          }
+
           case "declarator": {
@@ -510,11 +612,53 @@ const name = p<typeof n>("name")
             } else {
+              const layout = chooseAssignmentLayout(path, options, "init")
               const groupId = Symbol("declarator")
+              const leftDoc = name
+              const operator = " ="
+              const rightDoc = p<typeof n>("init")
               // TODO: need this group?
-              return group([
-                name,
-                arraySpecifier,
-                " =",
-                group(indent(line), { id: groupId }),
-                indentIfBreak(p<typeof n>("init"), { groupId }),
-              ])
+              // return group([
+              //   name,
+              //   arraySpecifier,
+              //   " =",
+              //   group(indent(line), { id: groupId }),
+              //   indentIfBreak(p<typeof n>("init"), { groupId }),
+              // ])
+              switch (layout) {
+                // First break after operator, then the sides are broken independently on their own lines
+                case "break-after-operator":
+                  return group([
+                    group(leftDoc),
+                    operator,
+                    group(indent([line, rightDoc])),
+                  ])
+
+                // First break right-hand side, then left-hand side
+                case "never-break-after-operator":
+                  return group([group(leftDoc), operator, " ", rightDoc])
+
+                // First break right-hand side, then after operator
+                case "fluid": {
+                  const groupId = Symbol("assignment")
+                  return group([
+                    group(leftDoc),
+                    operator,
+                    group(indent(line), { id: groupId }),
+                    lineSuffixBoundary,
+                    indentIfBreak(rightDoc, { groupId }),
+                  ])
+                }
+
+                // Parts of assignment chains aren't wrapped in groups.
+                // Once one of them breaks, the chain breaks too.
+                case "chain":
+                  return [group(leftDoc), operator, line, rightDoc]
+
+                case "chain-tail":
+                  return [group(leftDoc), operator, indent([line, rightDoc])]
+
+                case "only-left":
+                  return leftDoc
+                default:
+                  throw new Error()
+              }
             }
@@ -583,4 +727,8 @@ }
           case "returnStatement": {
-            const what = p<typeof n>("what")
-            return ["return", " ", what, ";"]
+            if (n.what) {
+              const what = p<typeof n>("what")
+              return ["return", " ", what, ";"]
+            } else {
+              return "return;"
+            }
             // return ["return", " ", conditionalGroup([what, indent(what)]), ";"]
@@ -692,4 +840,3 @@ }
               p<typeof n>("statement"),
-              "while",
-              "(",
+              "while (",
               group(p<typeof n>("conditionExpression")),
@@ -752,16 +899,31 @@ ")",
             const parts = printBinaryishExpressions(path, print, n, inMacro)
+            const parent = path.getParentNode() as Node
+            const isInsideParenthesis =
+              parent.kind === "selectionStatement" ||
+              parent.kind === "whileStatement" ||
+              parent.kind === "switchStatement" ||
+              parent.kind === "doWhileStatement"
 
-            // const q = vec2(
-            //   length(p.xz - 0.5 * b * vec2(1.0 - f, 1.0 + f)) *
-            //     sign(p.x * b.y + p.z * b.x - b.x * b.y) -
-            //     ra,
-            //   p.y - h,
-            // )
+            // if (
+            //   this.hasPlugin("dynamicImports") && this.lookahead().type === tt.parenLeft
+            // ) {
+            //
+            // looks super weird, we want to break the children if the parent breaks
+            //
+            // if (
+            //   this.hasPlugin("dynamicImports") &&
+            //   this.lookahead().type === tt.parenLeft
+            // ) {
+            if (isInsideParenthesis) {
+              return parts
+            }
 
             const needsParen = () => {
               const parent = path.getParentNode() as Node | undefined
-              // Comma expression passed as an argument to a function needs parentheses.
               if (!parent) {
                 return false
               }
+
+              // Comma expressions passed as arguments need parentheses.
+              // E.g. foo((a, b), c)
               if (
@@ -774,2 +936,4 @@ parent.kind === "functionCall" &&
 
+              // Bitwise operators are wrapped in parentheses for clarity.
+              // E.g. a << (u & 16)
               if (
@@ -784,5 +948,18 @@ parent.kind === "binaryExpression" &&
             }
-            const shouldIndent = true
 
-            if (shouldIndent) {
+            const parentParent = path.getParentNode(1) as Node
+
+            // Avoid indenting sub-expressions in some cases where the first sub-expression is already
+            // indented accordingly. We should indent sub-expressions where the first case isn't indented.
+            const shouldNotIndent =
+              parent.kind === "returnStatement" ||
+              parent.kind === "forStatement" ||
+              (parent.kind === "conditionalExpression" &&
+                parentParent.kind !== "returnStatement" &&
+                parentParent.kind !== "functionCall") ||
+              parent.kind === "assignmentExpression" ||
+              parent.kind === "declarator"
+            if (shouldNotIndent) {
+              return group(parts)
+            } else {
               // Separate the leftmost expression, possibly with its leading comments.
@@ -804,4 +981,2 @@ const headParts = parts.slice(0, 1)
               )
-            } else {
-              return group(parts)
             }
@@ -808,0 +983,0 @@ }

src/testutil/dedent.ts

@@ -37,6 +37,2 @@ /**
     result += raw[i]
-      // join lines when there is a suppressed newline
-      .replace(/\\\n[ \t]*/g, "")
-      // handle escaped backticks
-      .replace(/\\`/g, "`")
 
@@ -43,0 +39,0 @@ if (i < (args.length <= 1 ? 0 : args.length - 1)) {

No alert changes

Improved metrics

Total package byte prevSize

518876

increased by2.78%

Dependency count

5

decreased by-16.67%

Lines of code

11621

increased by3.11%

Number of lines in readme file

121

increased by1412.5%

Worsened metrics

Dev dependency count

20

increased by5.26%

Dependency changes

• - Removed@types/dedent@^0.7.0

• - Removed@types/dedent@0.7.2(transitive)