Extract getInterpreter() to module scope. Add another minor test.

prettier · ikatyang · Oct 5, 2018 · Sep 27, 2018 · Sep 27, 2018 · Sep 27, 2018
commit 096495dadd0ba439b2b975fed9caf1d08dd37b6c
diff --git a/src/main/options.js b/src/main/options.js
@@ -116,70 +116,55 @@ function getPlugin(options) {
   return printerPlugin;
 }
 
-function inferParser(filepath, plugins) {
-  const filepathParts = normalizePath(filepath).split("/");
-  const filename = filepathParts[filepathParts.length - 1].toLowerCase();
+function getInterpreter(filepath) {
+  if (typeof filepath !== "string") {
+    return "";
+  }
 
-  let interpreter = null;
-  const getInterpreter = () => {
-    if (interpreter === null) {
-      if (typeof filepath !== "string") {
-        interpreter = "";
-        return interpreter;
-      }
+  let fd;
+  try {
+    fd = fs.openSync(filepath, "r");
+  } catch (err) {
+    return "";
+  }
 
-      let fd;
-      try {
-        fd = fs.openSync(filepath, "r");
-      } catch (err) {
-        interpreter = "";
-        return interpreter;
-      }
+  try {
+    const liner = new readlines(fd);
+    const firstLine = liner
+      .next()
+      .toString("utf8")
+      .trim();
+
+    // #!/bin/env node, #!/usr/bin/env node
+    const m1 = /^#!\/usr(?:\/bin)?\/env\s+(\S+).*/.exec(firstLine);
+    if (m1) {
+      return m1[1];
+    }
 
-      try {
-        const liner = new readlines(fd);
-        const firstLine = liner
-          .next()
-          .toString("utf8")
-          .trim();
-
-        // #!/bin/env node
-        // #!/usr/bin/env node
-        const m1 = /^#!\/usr(?:\/bin)?\/env\s+(\S+).*/.exec(firstLine);
-        if (m1) {
-          interpreter = m1[1];
-          return interpreter;
-        }
-
-        // #!/bin/node
-        // #!/usr/bin/node
-        // #!/usr/local/bin/node
-        const m2 = /^#!\/usr(?:\/local)?(?:\/bin)?\/(\S+).*/.exec(firstLine);
-        if (m2) {
-          interpreter = m2[1];
-          return interpreter;
-        }
-
-        interpreter = "";
-        return interpreter;
-      } catch (err) {
-        // There are some weird cases where paths are missing, causing Jest
-        // failures. It's unclear what these correspond to in the real world.
-        interpreter = "";
-        return interpreter;
-      } finally {
-        try {
-          // There are some weird cases where paths are missing, causing Jest
-          // failures. It's unclear what these correspond to in the real world.
-          fs.closeSync(fd);
-        } catch (err) {
-          // nop
-        }
-      }
+    // #!/bin/node, #!/usr/bin/node, #!/usr/local/bin/node
+    const m2 = /^#!\/usr(?:\/local)?(?:\/bin)?\/(\S+).*/.exec(firstLine);
+    if (m2) {
+      return m2[1];
+    }
+    return "";
+  } catch (err) {
+    // There are some weird cases where paths are missing, causing Jest
+    // failures. It's unclear what these correspond to in the real world.
+    return "";
+  } finally {
+    try {
+      // There are some weird cases where paths are missing, causing Jest
+      // failures. It's unclear what these correspond to in the real world.
+      fs.closeSync(fd);
+    } catch (err) {
+      // nop
     }
+  }
+}
 
-    return interpreter;
-  };
+function inferParser(filepath, plugins) {
+  const filepathParts = normalizePath(filepath).split("/");
+  const filename = filepathParts[filepathParts.length - 1].toLowerCase();
 
   // If the file has no extension, we can try to infer the language from the
   // interpreter in the shebang line, if any; but since this requires FS access,
@@ -195,7 +180,7 @@ function inferParser(filepath, plugins) {
           language.filenames.find(name => name.toLowerCase() === filename)) ||
         (filename.indexOf(".") === -1 &&
           language.interpreters &&
-          language.interpreters.indexOf(getInterpreter()) !== -1))
+          language.interpreters.indexOf(getInterpreter(filepath)) !== -1))
   );
 
   return language && language.parsers[0];

diff --git a/tests_integration/__tests__/file-info.js b/tests_integration/__tests__/file-info.js
@@ -194,9 +194,18 @@ test("API getFileInfo with withNodeModules", () => {
   });
 });
 
-describe("extracts file-info for a file with no extension but a standard shebang", () => {
+describe("extracts file-info for a JS file with no extension but a standard shebang", () => {
   expect(
-    prettier.getFileInfo.sync("tests_integration/cli/shebang/sample-bin")
+    prettier.getFileInfo.sync("tests_integration/cli/shebang/sample-bin1")
+  ).toMatchObject({
+    ignored: false,
+    inferredParser: "babylon"
+  });
+});
+
+describe("extracts file-info for a JS file with no extension but an env-based shebang", () => {
+  expect(
+    prettier.getFileInfo.sync("tests_integration/cli/shebang/sample-bin2")
   ).toMatchObject({
     ignored: false,
     inferredParser: "babylon"

diff --git a/tests_integration/cli/shebang/sample-bin1 b/tests_integration/cli/shebang/sample-bin1
@@ -0,0 +1,2 @@
+#!/usr/bin/node
+'use strict';
diff --git a/tests_integration/cli/shebang/sample-bin → tests_integration/cli/shebang/sample-bin2 b/tests_integration/cli/shebang/sample-bin → tests_integration/cli/shebang/sample-bin2
diff --git a/tests_integration/cli/shebang/sample-bin3 b/tests_integration/cli/shebang/sample-bin3
@@ -0,0 +1 @@
+#!/usr/bin/php
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1 @@
		#!/usr/bin/php
haggholm marked this conversation as resolved. Outdated Show resolved Hide resolved