[Issue 17826] Fix ES Lint errors in common\tools\dev\tool (#18819)

* addded return types to functions

* added  to empty functions in test/framework.spec.ts

* fixed src/util/testProxyUtils.ts

* fixed src/util/checkWithTimeout.ts

* fixed type issue in common/tools/dev-tool/src/framework/CommandModule.ts

* fixed src/commands/test-proxy/start.ts & src/commands/test-proxy/waitForProxyEndpoint.ts

* fixed src/commands/test-proxy/start.ts & src/commands/test-proxy/waitForProxyEndpoint.ts

* Revert "fixed src/commands/test-proxy/start.ts & src/commands/test-proxy/waitForProxyEndpoint.ts"

This reverts commit fc8d7e1.

* fixed src/commands/samples/tsToJs.ts

* fixed src/commands/samples/run.ts

* fixed src/commands/samples/publish.ts

* fixed src/commands/samples/prep.ts

* fixed src/commands/samples/checkNodeVersions.ts

* fixed unassignbale type casting between string and unknown

* fixed non null assertion error and removed arguments from package.json:scripts:lint

* fixed dynamic import and other minor issues

* reverted back to require and suppressed the error in src\commands\samples\tsToJs.ts

* removed redundant type casting

common/tools/dev-tool/package.json

@@ -20,7 +20,7 @@
     "integration-test:browser": "echo skipped",
     "integration-test:node": "echo skipped",
     "lint:fix": "eslint --no-eslintrc -c ../../.eslintrc.internal.json package.json src test --ext .ts --fix --fix-type [problem,suggestion]",
-    "lint": "eslint --no-eslintrc -c ../../../sdk/.eslintrc.internal.json package.json src test --ext .ts  -f html -o template-lintReport.html || exit 0",
+    "lint": "eslint --no-eslintrc -c ../../../sdk/.eslintrc.internal.json package.json src test --ext .ts",
     "pack": "npm pack 2>&1",
     "prebuild": "npm run clean",
     "unit-test": "npm run unit-test:node",

common/tools/dev-tool/src/commands/index.ts

@@ -25,7 +25,7 @@ export const baseCommandInfo = makeCommandInfo("dev-tool", "Azure SDK for JS dev
 /**
  * Default dev-tool subcommand
  */
-export const baseCommand = async (...args: string[]) => {
+export const baseCommand = async (...args: string[]): Promise<void> => {
   const status = await subCommand(baseCommandInfo, baseCommands)(...args);
 
   if (!status) {

common/tools/dev-tool/src/commands/samples/checkNodeVersions.ts

@@ -87,30 +87,30 @@ function buildRunSamplesScript(
   const scriptContent = `#!/bin/sh
 
 function install_dependencies_helper() {
-  local samples_path=\$1;
+  local samples_path=$1;
   cd \${samples_path};
   ${compileCMD(`npm install ${artifactURL}`, printToScreen)}
   ${compileCMD(`npm install`, printToScreen)}
 }
 
 function install_packages() {
-  echo "Using node \$(node -v) to install dependencies";
+  echo "Using node $(node -v) to install dependencies";
   install_dependencies_helper ${samplesDir}/javascript
   install_dependencies_helper ${samplesDir}/typescript;
   cp ${envFilePath} ${samplesDir}/javascript/;
 }
 
 function run_samples() {
-  samples_path=\$1;
-  echo "Using node \$(node -v) to run samples in \${samples_path}";
+  samples_path=$1;
+  echo "Using node $(node -v) to run samples in \${samples_path}";
   cd "\${samples_path}";
   for SAMPLE in *.js; do
     node \${SAMPLE};
   done
 }
 
 function build_typescript() {
-  echo "Using node \$(node -v) to build the typescript samples";
+  echo "Using node $(node -v) to build the typescript samples";
   cd ${samplesDir}/typescript
   ${compileCMD(`npm run build`, printToScreen)}
   cp ${envFilePath} ${samplesDir}/typescript/dist/
@@ -273,7 +273,7 @@ export default leafCommand(commandInfo, async (options) => {
       options["node-versions"]
         ?.split(",")
         .concat(options["node-version"])
-        .filter((ver) => ver !== "" && parseInt(ver) !== NaN)
+        .filter((ver) => ver !== "" && !isNaN(parseInt(ver)))
     )
   ];
   const dockerContextDirectory: string =

common/tools/dev-tool/src/commands/samples/prep.ts

@@ -66,7 +66,7 @@ async function enableLocalRun(
     // that can escape both linux and windows path separators
     const depth = relativeDir.length - relativeDir.split(path.sep).join("").length;
 
-    let relativePath = new Array(depth).fill("..").join("/");
+    const relativePath = new Array(depth).fill("..").join("/");
 
     outputContent = fileContents.replace(
       importRegex,

common/tools/dev-tool/src/commands/samples/publish.ts

@@ -81,12 +81,12 @@ function createPackageJson(info: SampleGenerationInfo, outputKind: OutputKind):
     },
     ...(outputKind === OutputKind.TypeScript
       ? {
-          // We only include these in TypeScript
-          scripts: {
-            build: "tsc",
-            prebuild: "rimraf dist/"
-          }
+        // We only include these in TypeScript
+        scripts: {
+          build: "tsc",
+          prebuild: "rimraf dist/"
         }
+      }
       : {}),
     repository: {
       type: "git",
@@ -122,7 +122,7 @@ function isDependency(moduleSpecifier: string): boolean {
 
   // This seems like a reasonable test for "is a relative path" as long as
   // absolute path imports are forbidden.
-  const isRelativePath = /^\.\.?[\/\\]/.test(moduleSpecifier);
+  const isRelativePath = /^\.\.?[/\\]/.test(moduleSpecifier);
   return !isRelativePath;
 }
 
@@ -372,10 +372,14 @@ async function makeSampleGenerationInfo(
     // Resolve snippets to actual text
     customSnippets: Object.entries(sampleConfiguration.customSnippets ?? {}).reduce(
       (accum, [name, file]) => {
+        if (!file) {
+          return accum;
+        }
+
         let contents;
 
         try {
-          contents = fs.readFileSync(file!);
+          contents = fs.readFileSync(file);
         } catch (ex) {
           fail(`Failed to read custom snippet file '${file}'`, ex);
         }
@@ -430,14 +434,14 @@ async function makeSampleGenerationInfo(
         }, defaultDependencies),
         ...(outputKind === OutputKind.TypeScript
           ? {
-              // In TypeScript samples, we include TypeScript and `rimraf`, because they're used
-              // in the package scripts.
-              devDependencies: {
-                ...typesDependencies,
-                typescript: devToolPackageJson.dependencies.typescript,
-                rimraf: "latest"
-              }
+            // In TypeScript samples, we include TypeScript and `rimraf`, because they're used
+            // in the package scripts.
+            devDependencies: {
+              ...typesDependencies,
+              typescript: devToolPackageJson.dependencies.typescript,
+              rimraf: "latest"
             }
+          }
           : {})
       };
     }
@@ -455,11 +459,11 @@ function createReadme(outputKind: OutputKind, info: SampleGenerationInfo): strin
     frontmatter: info.disableDocsMs
       ? undefined
       : {
-          page_type: "sample",
-          languages: [fullOutputKind],
-          products: info.productSlugs,
-          urlFragment: `${info.baseName}-${fullOutputKind}`
-        },
+        page_type: "sample",
+        languages: [fullOutputKind],
+        products: info.productSlugs,
+        urlFragment: `${info.baseName}-${fullOutputKind}`
+      },
     publicationDirectory: PUBLIC_SAMPLES_BASE + "/" + info.topLevelDirectory,
     useTypeScript: outputKind === OutputKind.TypeScript,
     ...info,

common/tools/dev-tool/src/commands/samples/run.ts

@@ -28,10 +28,10 @@ export const commandInfo = makeCommandInfo(
 async function runSingle(name: string, accumulatedErrors: Array<[string, string]>) {
   log("Running", name);
   try {
-    if (/.*[\\\/]samples(-dev)?[\\\/].*/.exec(name)) {
+    if (/.*[\\/]samples(-dev)?[\\/].*/.exec(name)) {
       // This is an un-prepared sample, so just require it and it will run.
       await import(name);
-    } else if (!/.*[\\\/]dist-samples[\\\/].*/.exec(name)) {
+    } else if (!/.*[\\/]dist-samples[\\/].*/.exec(name)) {
       // This is not an unprepared or a prepared sample
       log.error("Skipped. This file is not in any samples folder.");
     } else {

common/tools/dev-tool/src/commands/samples/tsToJs.ts

@@ -20,6 +20,7 @@ export const commandInfo = makeCommandInfo(
 );
 
 const prettierOptions: prettier.Options = {
+  // eslint-disable-next-line @typescript-eslint/no-var-requires
   ...(require("../../../../eslint-plugin-azure-sdk/prettier.json") as prettier.Options),
   parser: "typescript"
 };

common/tools/dev-tool/src/commands/test-proxy/start.ts

@@ -12,7 +12,7 @@ export const commandInfo = makeCommandInfo(
   {}
 );
 
-export default leafCommand(commandInfo, async (_options) => {
+export default leafCommand(commandInfo, async () => {
   await startProxyTool();
   return true;
 });

common/tools/dev-tool/src/commands/test-proxy/waitForProxyEndpoint.ts

@@ -13,7 +13,7 @@ export const commandInfo = makeCommandInfo(
   {}
 );
 
-export default leafCommand(commandInfo, async (_options) => {
+export default leafCommand(commandInfo, async () => {
   const result = await checkWithTimeout(isProxyToolActive, 1000, 120000);
   return result;
 });

common/tools/dev-tool/src/config/rollup.base.config.ts

@@ -46,15 +46,15 @@ export function openTelemetryCommonJs(): Record<string, string[]> {
       // working around a limitation in the rollup common.js plugin - it's not able to resolve these modules so the named exports listed above will not get applied. We have to drill down to the actual path.
       `../../../common/temp/node_modules/.pnpm/@opentelemetry+api@${version}/node_modules/@opentelemetry/api/build/src/index.js`
     ] = [
-      "SpanKind",
-      "TraceFlags",
-      "getSpan",
-      "setSpan",
-      "StatusCode",
-      "CanonicalCode",
-      "getSpanContext",
-      "setSpanContext"
-    ];
+        "SpanKind",
+        "TraceFlags",
+        "getSpan",
+        "setSpan",
+        "StatusCode",
+        "CanonicalCode",
+        "getSpanContext",
+        "setSpanContext"
+      ];
   }
 
   return namedExports;
@@ -72,7 +72,7 @@ function ignoreNiseSinonEvalWarnings(warning: RollupWarning): boolean {
   return (
     warning.code === "EVAL" &&
     (warning.id?.includes("node_modules/nise") || warning.id?.includes("node_modules/sinon")) ===
-      true
+    true
   );
 }
 
@@ -103,7 +103,7 @@ function makeOnWarnForTesting(): (warning: RollupWarning, warn: WarningHandler)
 
 // #endregion
 
-export function makeBrowserTestConfig() {
+export function makeBrowserTestConfig(): RollupOptions {
   const config: RollupOptions = {
     input: {
       include: ["dist-esm/test/**/*.spec.js"],
@@ -151,7 +151,7 @@ const defaultConfigurationOptions: ConfigurationOptions = {
   disableBrowserBundle: false
 };
 
-export function makeConfig(pkg: PackageJson, options?: Partial<ConfigurationOptions>) {
+export function makeConfig(pkg: PackageJson, options?: Partial<ConfigurationOptions>): RollupOptions[] {
   options = {
     ...defaultConfigurationOptions,
     ...(options ?? {})

common/tools/dev-tool/src/framework/CommandModule.ts

@@ -25,4 +25,4 @@ export interface CommandModule<Options extends CommandOptions> {
 /**
  * A map from command name to an async function that loads its module
  */
-export type CommandLoader = { [k: string]: () => Promise<CommandModule<{}>> };
+export type CommandLoader = { [k: string]: () => Promise<CommandModule<CommandOptions>> };

common/tools/dev-tool/src/templates/sampleReadme.md.ts

@@ -84,9 +84,8 @@ function resourceLinks(info: SampleReadmeConfiguration) {
 function resources(info: SampleReadmeConfiguration) {
   const resources = Object.entries(info.requiredResources ?? {});
 
-  const header = `You need [an Azure subscription][freesub] ${
-    resources.length > 0 ? "and the following Azure resources " : ""
-  }to run these sample programs${resources.length > 0 ? ":\n\n" : "."}`;
+  const header = `You need [an Azure subscription][freesub] ${resources.length > 0 ? "and the following Azure resources " : ""
+    }to run these sample programs${resources.length > 0 ? ":\n\n" : "."}`;
 
   return (
     header + resources.map(([name]) => `- [${name}][${resourceNameToLinkSlug(name)}]`).join("\n")
@@ -137,15 +136,14 @@ function exampleNodeInvocation(info: SampleReadmeConfiguration) {
     .map((envVar) => `${envVar}="<${envVar.replace(/_/g, " ").toLowerCase()}>"`)
     .join(" ");
 
-  return `${envVars} node ${
-    info.useTypeScript ? "dist/" : ""
-  }${firstModule.relativeSourcePath.replace(/\.ts$/, ".js")}`;
+  return `${envVars} node ${info.useTypeScript ? "dist/" : ""
+    }${firstModule.relativeSourcePath.replace(/\.ts$/, ".js")}`;
 }
 
 /**
  * Creates a README for a sample package from a SampleReadmeConfiguration.
  */
-export default (info: SampleReadmeConfiguration) => {
+export default (info: SampleReadmeConfiguration): string => {
   let stepCount = 1;
   const step = (content: string) => `${stepCount++}. ${content}`;
 
@@ -157,8 +155,7 @@ export default (info: SampleReadmeConfiguration) => {
 
 ${info.customSnippets?.header ?? ""}
 
-These sample programs show how to use the ${language} client libraries for ${
-      info.productName
+These sample programs show how to use the ${language} client libraries for ${info.productName
     } in some common scenarios.
 
 ${table(info)}
@@ -168,17 +165,17 @@ ${table(info)}
 The sample programs are compatible with [LTS versions of Node.js](https://nodejs.org/about/releases/).
 
 ${(() => {
-  if (info.useTypeScript) {
-    return [
-      "Before running the samples in Node, they must be compiled to JavaScript using the TypeScript compiler. For more information on TypeScript, see the [TypeScript documentation][typescript]. Install the TypeScript compiler using:",
-      "",
-      fence("bash", "npm install -g typescript"),
-      ""
-    ].join("\n");
-  } else {
-    return "";
-  }
-})()}\
+      if (info.useTypeScript) {
+        return [
+          "Before running the samples in Node, they must be compiled to JavaScript using the TypeScript compiler. For more information on TypeScript, see the [TypeScript documentation][typescript]. Install the TypeScript compiler using:",
+          "",
+          fence("bash", "npm install -g typescript"),
+          ""
+        ].join("\n");
+      } else {
+        return "";
+      }
+    })()}\
 ${resources(info)}
 
 ${info.customSnippets?.prerequisites ?? ""}
@@ -195,28 +192,28 @@ ${step("Install the dependencies using `npm`:")}
 
 ${fence("bash", "npm install")}
 ${(() => {
-  if (info.useTypeScript) {
-    return [step("Compile the samples:"), "", fence("bash", "npm run build"), ""].join("\n");
-  } else {
-    return "";
-  }
-})()}
+      if (info.useTypeScript) {
+        return [step("Compile the samples:"), "", fence("bash", "npm run build"), ""].join("\n");
+      } else {
+        return "";
+      }
+    })()}
 ${step(
-  "Edit the file `sample.env`, adding the correct credentials to access the Azure service and run the samples. Then rename the file from `sample.env` to just `.env`. The sample programs will read this file automatically."
-)}
+      "Edit the file `sample.env`, adding the correct credentials to access the Azure service and run the samples. Then rename the file from `sample.env` to just `.env`. The sample programs will read this file automatically."
+    )}
 
 ${step(
-  "Run whichever samples you like (note that some samples may require additional setup, see the table above):"
-)}
+      "Run whichever samples you like (note that some samples may require additional setup, see the table above):"
+    )}
 
 ${fence(
-  "bash",
-  `node ${(() => {
-    const firstSource = filterModules(info)[0].relativeSourcePath;
-    const filePath = info.useTypeScript ? "dist/" : "";
-    return filePath + firstSource.replace(/\.ts$/, ".js");
-  })()}`
-)}
+      "bash",
+      `node ${(() => {
+        const firstSource = filterModules(info)[0].relativeSourcePath;
+        const filePath = info.useTypeScript ? "dist/" : "";
+        return filePath + firstSource.replace(/\.ts$/, ".js");
+      })()}`
+    )}
 
 Alternatively, run a single sample with the correct environment variables set (setting up the \`.env\` file is not required if you do this), for example (cross-platform):
 

common/tools/dev-tool/src/util/checkWithTimeout.ts

@@ -10,8 +10,8 @@ const log = createPrinter("check-with-timeout");
  */
 export async function checkWithTimeout(
   predicate: () => boolean | Promise<boolean>,
-  delayBetweenRetriesInMilliseconds: number = 1000,
-  maxWaitTimeInMilliseconds: number = 10000
+  delayBetweenRetriesInMilliseconds = 1000,
+  maxWaitTimeInMilliseconds = 10000
 ): Promise<boolean> {
   const maxTime = Date.now() + maxWaitTimeInMilliseconds;
   while (Date.now() < maxTime) {

common/tools/dev-tool/src/util/findMatchingFiles.ts

@@ -57,7 +57,7 @@ export async function* findMatchingFiles(
   dir: string,
   matches: (name: string, entry: fs.Stats) => boolean,
   findOptions?: Partial<FindOptions>
-) {
+): AsyncGenerator<string> {
   const q: FileInfo[] = [];
 
   const options: FindOptions = { ...defaultFindOptions, ...findOptions };

common/tools/dev-tool/src/util/sampleConfiguration.ts

@@ -142,7 +142,7 @@ const removeJsTsExtensions = (name: string): string => name.replace(/\.[jt]s$/,
  * @param info - FileInfo of a sample file to be considered
  * @param skips - a list of strings that identify files to be skipped
  */
-export function shouldSkip(info: FileInfo, skips: string[]) {
+export function shouldSkip(info: FileInfo, skips: string[]): boolean {
   // Add a slash to the skip if necessary
   const addFirstSlash = (skip: string): string => (skip.startsWith("/") ? skip : "/" + skip);