[dev-tool] Don't allow external default imports

common/tools/dev-tool/src/util/samples/generation.ts

@@ -20,6 +20,7 @@ import { processSources } from "./processor";
 
 import devToolPackageJson from "../../../package.json";
 import instantiateSampleReadme from "../../templates/sampleReadme.md";
+import { resolveModule } from "./transforms";
 
 const log = createPrinter("generator");
 
@@ -62,21 +63,6 @@ export function createPackageJson(info: SampleGenerationInfo, outputKind: Output
   };
 }
 
-/**
- * Processes a segmented module path to return the first segment. This is useful for packages that have nested imports
- * such as "dayjs/plugin/duration".
- *
- * @param specifier - the module specifier to resolve to a package name
- * @returns a package name
- */
-function resolveModule(specifier: string): string {
-  const parts = specifier.split("/", 2);
-
-  // The first part could be a namespace, in which case we need to join them
-  if (parts.length > 1 && parts[0].startsWith("@")) return parts[0] + "/" + parts[1];
-  else return parts[0];
-}
-
 async function collect<T>(i: AsyncIterableIterator<T>): Promise<T[]> {
   const out = [];
 

common/tools/dev-tool/src/util/samples/processor.ts

@@ -1,7 +1,6 @@
 // Copyright (c) Microsoft Corporation.
 // Licensed under the MIT license.
 
-import nodeBuiltins from "builtin-modules";
 import fs from "fs-extra";
 import path from "path";
 import * as ts from "typescript";
@@ -11,7 +10,7 @@ import { createAccumulator } from "../typescript/accumulator";
 import { createDiagnosticEmitter } from "../typescript/diagnostic";
 import { AzSdkMetaTags, AZSDK_META_TAG_PREFIX, ModuleInfo, VALID_AZSDK_META_TAGS } from "./info";
 import { testSyntax } from "./syntax";
-import { toCommonJs } from "./transforms";
+import { isDependency, isRelativePath, toCommonJs } from "./transforms";
 
 const log = createPrinter("samples:processor");
 
@@ -381,25 +380,3 @@ function processExportDefault(
         );
   });
 }
-
-/**
- * Determines whether a module specifier is a package dependency.
- *
- * A dependency is a module specifier that does not refer to a node builtin and
- * is not a relative path.
- *
- * Absolute path imports are not supported in samples (because the package base
- * is not fixed relative to the source file).
- *
- * @param moduleSpecifier - the string given to `import` or `require`
- * @returns - true if `moduleSpecifier` should be considered a reference to a
- * node module dependency
- */
-function isDependency(moduleSpecifier: string): boolean {
-  if (nodeBuiltins.includes(moduleSpecifier)) return false;
-
-  return !isRelativePath(moduleSpecifier);
-}
-
-// This seems like a reasonable test for "is a relative path"
-const isRelativePath = (path: string) => /^\.\.?[/\\]/.test(path);

common/tools/dev-tool/src/util/samples/syntax.ts

@@ -3,6 +3,7 @@
 
 import * as ts from "typescript";
 import { createPrinter } from "../printer";
+import { isNodeBuiltin, isRelativePath } from "./transforms";
 
 const log = createPrinter("samples:syntax");
 
@@ -46,6 +47,21 @@ const SYNTAX_VIABILITY_TESTS = {
     // import("foo")
     ImportExpression: (node: ts.Node) =>
       ts.isCallExpression(node) && node.expression.kind === ts.SyntaxKind.ImportKeyword,
+    // This can't be supported without going to great lengths to emulate esModuleInterop behavior.
+    // It's a little more involved to test for. We only care about `import <name> from <specifier>`
+    // where <specifier> does not refer to a builtin or a relative module path.
+    ExternalDefaultImport: (node: ts.Node) => {
+      const isDefaultImport =
+        ts.isImportDeclaration(node) &&
+        node.importClause?.name &&
+        ts.isIdentifier(node.importClause.name);
+
+      if (!isDefaultImport) return false;
+
+      const { text: moduleSpecifier } = node.moduleSpecifier as ts.StringLiteralLike;
+
+      return isDefaultImport && !isNodeBuiltin(moduleSpecifier) && !isRelativePath(moduleSpecifier);
+    },
   },
   // Supported in Node 14+
   ES2020: {

common/tools/dev-tool/src/util/samples/transforms.ts

@@ -3,6 +3,8 @@
 
 import ts from "typescript";
 
+import nodeBuiltins from "builtin-modules";
+
 /**
  * A TypeScript API transformer that replaces imports with CommonJS `require` calls.
  *
@@ -87,6 +89,14 @@ export function importDeclarationToCommonJs(
   const primaryBinding = importClauseToBinding(decl.importClause, factory);
 
   const namedBindings = decl.importClause.namedBindings;
+  const moduleSpecifierText = (decl.moduleSpecifier as ts.StringLiteral).text;
+
+  const isDefaultImport =
+    ts.isIdentifier(primaryBinding) &&
+    // We only allow default imports on relative modules and node builtins, but on node builtins they are actually
+    // just namespace imports in disguise. This is because of esModuleInterop compatibility in our tsconfig.json.
+    !isNodeBuiltin(moduleSpecifierText) &&
+    (!namedBindings || !ts.isNamespaceImport(namedBindings));
 
   // The declaration will usually only contain one item, and it will be something like:
   //
@@ -102,7 +112,7 @@ export function importDeclarationToCommonJs(
       /* exclamationToken: */ undefined,
       /* type: */ undefined,
       // If the binding was a name, and this isn't a namespace import, then we need to access .default on it.
-      ts.isIdentifier(primaryBinding) && (!namedBindings || !ts.isNamespaceImport(namedBindings))
+      isDefaultImport
         ? factory.createPropertyAccessExpression(requireCall(), "default")
         : requireCall()
     ),
@@ -176,3 +186,55 @@ function namedImportsToObjectBindingPattern(
     )
   );
 }
+
+/**
+ * Processes a segmented module path to return the first segment. This is useful for packages that have nested imports
+ * such as "dayjs/plugin/duration".
+ *
+ * @param specifier - the module specifier to resolve to a package name
+ * @returns a package name
+ */
+export function resolveModule(specifier: string): string {
+  const parts = specifier.split("/", 2);
+
+  // The first part could be a namespace, in which case we need to join them
+  if (parts.length > 1 && parts[0].startsWith("@")) return parts[0] + "/" + parts[1];
+  else return parts[0];
+}
+
+/**
+ * Determines if a module specifier refers to a node builtin.
+ *
+ * @param specifier - the module specifier to test
+ */
+export function isNodeBuiltin(moduleSpecifier: string): boolean {
+  return (
+    moduleSpecifier.startsWith("node:") || nodeBuiltins.includes(resolveModule(moduleSpecifier))
+  );
+}
+
+/**
+ * Determines whether a string is a relative path.
+ *
+ * @param input - a string to test
+ */
+export const isRelativePath = (input: string): boolean => /^\.\.?[/\\]/.test(input);
+
+/**
+ * Determines whether a module specifier is a package dependency.
+ *
+ * A dependency is a module specifier that does not refer to a node builtin and
+ * is not a relative path.
+ *
+ * Absolute path imports are not supported in samples (because the package base
+ * is not fixed relative to the source file).
+ *
+ * @param moduleSpecifier - the string given to `import` or `require`
+ * @returns - true if `moduleSpecifier` should be considered a reference to a
+ * node module dependency
+ */
+export function isDependency(moduleSpecifier: string): boolean {
+  if (isNodeBuiltin(moduleSpecifier)) return false;
+
+  return !isRelativePath(moduleSpecifier);
+}

common/tools/dev-tool/test/samples/files/expectations/cjs-forms/javascript/index.js

@@ -20,6 +20,10 @@ const Anonymous = require("./defaultExportsClass").default;
 
 require("./hasSideEffects");
 
+// Test builtins
+const path_1 = require("path");
+const path_2 = require("path");
+
 async function main() {
   const waitTime = process.env.WAIT_TIME || "5000";
   const delayMs = parseInt(waitTime);
@@ -37,6 +41,9 @@ async function main() {
 
   const object2 = new base.default(base.default.name);
   object2.say();
+
+  console.log("The path separator is:", path_1.sep);
+  console.log("And by a default import, it is also:", path_2.sep);
 }
 
 main().catch((error) => {

common/tools/dev-tool/test/samples/files/expectations/cjs-forms/typescript/src/index.ts

@@ -19,6 +19,10 @@ import Anonymous from "./defaultExportsClass";
 
 import "./hasSideEffects";
 
+// Test builtins
+import * as path_1 from "path";
+import path_2 from "path";
+
 async function main() {
   const waitTime = process.env.WAIT_TIME || "5000";
   const delayMs = parseInt(waitTime);
@@ -36,6 +40,9 @@ async function main() {
 
   const object2 = new base.default(base.default.name);
   object2.say();
+
+  console.log("The path separator is:", path_1.sep);
+  console.log("And by a default import, it is also:", path_2.sep);
 }
 
 main().catch((error) => {

common/tools/dev-tool/test/samples/files/inputs/cjs-forms/index.ts

@@ -19,6 +19,10 @@ import Anonymous from "./defaultExportsClass";
 
 import "./hasSideEffects";
 
+// Test builtins
+import * as path_1 from "path";
+import path_2 from "path";
+
 async function main() {
   const waitTime = process.env.WAIT_TIME || "5000";
   const delayMs = parseInt(waitTime);
@@ -36,6 +40,9 @@ async function main() {
 
   const object2 = new base.default(base.default.name);
   object2.say();
+
+  console.log("The path separator is:", path_1.sep);
+  console.log("And by a default import, it is also:", path_2.sep);
 }
 
 main().catch((error) => {

sdk/template/template/samples-dev/getConfigurationSetting.ts

@@ -12,7 +12,7 @@ import { DefaultAzureCredential } from "@azure/identity";
 import * as dotenv from "dotenv";
 dotenv.config();
 
-export async function main() {
+async function main() {
   const endpoint = process.env.APPCONFIG_ENDPOINT || "<endpoint>";
   const key = process.env.APPCONFIG_TEST_SETTING_KEY || "<test-key>";
 

sdk/template/template/samples/v1-beta/typescript/src/getConfigurationSetting.ts

@@ -12,7 +12,7 @@ import { DefaultAzureCredential } from "@azure/identity";
 import * as dotenv from "dotenv";
 dotenv.config();
 
-export async function main() {
+async function main() {
   const endpoint = process.env.APPCONFIG_ENDPOINT || "<endpoint>";
   const key = process.env.APPCONFIG_TEST_SETTING_KEY || "<test-key>";