ref: Enable explicit-function-return-type eslint rule (#742)

* ref: Enable `explicit-function-return-type` eslint rule

* better solution for rollup-vite type conflicts

* cleanup

Author: Lms24

packages/babel-plugin-component-annotate/src/index.ts

@@ -152,7 +152,7 @@ function functionBodyPushAttributes(
   sourceFileName: string | undefined,
   attributeNames: string[],
   ignoredComponents: string[]
-) {
+): void {
   let jsxNode: Babel.NodePath;
 
   const functionBody = path.get("body").get("body");
@@ -248,7 +248,7 @@ function processJSX(
   sourceFileName: string | undefined,
   attributeNames: string[],
   ignoredComponents: string[]
-) {
+): void {
   if (!jsxNode) {
     return;
   }
@@ -323,7 +323,7 @@ function applyAttributes(
   sourceFileName: string | undefined,
   attributeNames: string[],
   ignoredComponents: string[]
-) {
+): void {
   const [componentAttributeName, elementAttributeName, sourceFileAttributeName] = attributeNames;
 
   if (isReactFragment(t, openingElement)) {
@@ -390,7 +390,7 @@ function applyAttributes(
   }
 }
 
-function sourceFileNameFromState(state: AnnotationPluginPass) {
+function sourceFileNameFromState(state: AnnotationPluginPass): string | undefined {
   const name = fullSourceFileNameFromState(state);
   if (!name) {
     return undefined;
@@ -416,7 +416,7 @@ function fullSourceFileNameFromState(state: AnnotationPluginPass): string | null
   return null;
 }
 
-function isKnownIncompatiblePluginFromState(state: AnnotationPluginPass) {
+function isKnownIncompatiblePluginFromState(state: AnnotationPluginPass): boolean {
   const fullSourceFileName = fullSourceFileNameFromState(state);
 
   if (!fullSourceFileName) {

packages/bundler-plugin-core/src/build-plugin-manager.ts

@@ -172,7 +172,7 @@ export function createSentryBuildPluginManager(
 
   let sessionHasEnded = false; // Just to prevent infinite loops with beforeExit, which is called whenever the event loop empties out
 
-  function endSession() {
+  function endSession(): void {
     if (sessionHasEnded) {
       return;
     }
@@ -206,7 +206,7 @@ export function createSentryBuildPluginManager(
    * or continue up to them. By default, @param throwByDefault controls if the plugin
    * should throw an error (which causes a build fail in most bundlers) or continue.
    */
-  function handleRecoverableError(unknownError: unknown, throwByDefault: boolean) {
+  function handleRecoverableError(unknownError: unknown, throwByDefault: boolean): void {
     sentrySession.status = "abnormal";
     try {
       if (options.errorHandler) {
@@ -251,13 +251,13 @@ export function createSentryBuildPluginManager(
   const dependenciesOnBuildArtifacts = new Set<symbol>();
   const buildArtifactsDependencySubscribers: (() => void)[] = [];
 
-  function notifyBuildArtifactDependencySubscribers() {
+  function notifyBuildArtifactDependencySubscribers(): void {
     buildArtifactsDependencySubscribers.forEach((subscriber) => {
       subscriber();
     });
   }
 
-  function createDependencyOnBuildArtifacts() {
+  function createDependencyOnBuildArtifacts(): () => void {
     const dependencyIdentifier = Symbol();
     dependenciesOnBuildArtifacts.add(dependencyIdentifier);
 
@@ -273,7 +273,7 @@ export function createSentryBuildPluginManager(
    * It is very important that this function is called as late as possible before wanting to await the Promise to give
    * the dependency producers as much time as possible to register themselves.
    */
-  function waitUntilBuildArtifactDependenciesAreFreed() {
+  function waitUntilBuildArtifactDependenciesAreFreed(): Promise<void> {
     return new Promise<void>((resolve) => {
       buildArtifactsDependencySubscribers.push(() => {
         if (dependenciesOnBuildArtifacts.size === 0) {
@@ -607,7 +607,7 @@ export function createSentryBuildPluginManager(
                     }
                   );
                   const workers: Promise<void>[] = [];
-                  const worker = async () => {
+                  const worker = async (): Promise<void> => {
                     while (preparationTasks.length > 0) {
                       const task = preparationTasks.shift();
                       if (task) {

packages/bundler-plugin-core/src/debug-id-upload.ts

@@ -26,7 +26,7 @@ export async function prepareBundleForDebugIdUpload(
   logger: Logger,
   rewriteSourcesHook: RewriteSourcesHook,
   resolveSourceMapHook: ResolveSourceMapHook | undefined
-) {
+): Promise<void> {
   let bundleContent;
   try {
     bundleContent = await promisify(fs.readFile)(bundleFilePath, "utf8");

packages/bundler-plugin-core/src/index.ts

@@ -4,19 +4,17 @@ import SentryCli from "@sentry/cli";
 import { logger } from "@sentry/utils";
 import * as fs from "fs";
 import { glob } from "glob";
-import MagicString from "magic-string";
+import MagicString, { SourceMap } from "magic-string";
 import * as path from "path";
-import { createUnplugin, TransformResult, UnpluginOptions } from "unplugin";
+import { createUnplugin, TransformResult, UnpluginInstance, UnpluginOptions } from "unplugin";
 import { createSentryBuildPluginManager } from "./build-plugin-manager";
 import { createDebugIdUploadFunction } from "./debug-id-upload";
 import { Logger } from "./logger";
 import { Options, SentrySDKBuildFlags } from "./types";
 import {
   generateGlobalInjectorCode,
   generateModuleMetadataInjectorCode,
-  getDependencies,
-  getPackageJson,
-  parseMajorVersion,
+  getBuildInformation as actualGetBuildInformation,
   replaceBooleanFlagsInCode,
   stringToUUID,
   stripQueryAndHashFromPath,
@@ -46,7 +44,7 @@ export function sentryUnpluginFactory({
   debugIdInjectionPlugin,
   debugIdUploadPlugin,
   bundleSizeOptimizationsPlugin,
-}: SentryUnpluginFactoryOptions) {
+}: SentryUnpluginFactoryOptions): UnpluginInstance<Options | undefined, true> {
   return createUnplugin<Options | undefined, true>((userOptions = {}, unpluginMetaContext) => {
     const sentryBuildPluginManager = createSentryBuildPluginManager(userOptions, {
       loggerPrefix:
@@ -177,22 +175,11 @@ export function sentryUnpluginFactory({
 }
 
 /**
- * @deprecated
+ * @deprecated This will be removed in v4
  */
-// TODO(v4): Don't export this from the package
-export function getBuildInformation() {
-  const packageJson = getPackageJson();
-
-  const { deps, depsVersions } = packageJson
-    ? getDependencies(packageJson)
-    : { deps: [], depsVersions: {} };
-
-  return {
-    deps,
-    depsVersions,
-    nodeVersion: parseMajorVersion(process.version),
-  };
-}
+// TODO(v4): Don't export this from the package but keep the utils version
+// eslint-disable-next-line @typescript-eslint/explicit-function-return-type
+export const getBuildInformation = actualGetBuildInformation;
 
 /**
  * Determines whether the Sentry CLI binary is in its expected location.
@@ -203,7 +190,11 @@ export function sentryCliBinaryExists(): boolean {
   return fs.existsSync(SentryCli.getPath());
 }
 
-export function createRollupReleaseInjectionHooks(injectionCode: string) {
+export function createRollupReleaseInjectionHooks(injectionCode: string): {
+  resolveId: UnpluginOptions["resolveId"];
+  load: UnpluginOptions["load"];
+  transform: UnpluginOptions["transform"];
+} {
   const virtualReleaseInjectionFileId = "\0sentry-release-injection-file";
   return {
     resolveId(id: string) {
@@ -260,7 +251,9 @@ export function createRollupReleaseInjectionHooks(injectionCode: string) {
   };
 }
 
-export function createRollupBundleSizeOptimizationHooks(replacementValues: SentrySDKBuildFlags) {
+export function createRollupBundleSizeOptimizationHooks(replacementValues: SentrySDKBuildFlags): {
+  transform: UnpluginOptions["transform"];
+} {
   return {
     transform(code: string) {
       return replaceBooleanFlagsInCode(code, replacementValues);
@@ -274,7 +267,24 @@ const COMMENT_USE_STRICT_REGEX =
   // Note: CodeQL complains that this regex potentially has n^2 runtime. This likely won't affect realistic files.
   /^(?:\s*|\/\*(?:.|\r|\n)*?\*\/|\/\/.*[\n\r])*(?:"[^"]*";|'[^']*';)?/;
 
-export function createRollupDebugIdInjectionHooks() {
+/**
+ * Simplified `renderChunk` hook type from Rollup.
+ * We can't reference the type directly because the Vite plugin complains
+ * about type mismatches
+ */
+type RenderChunkHook = (
+  code: string,
+  chunk: {
+    fileName: string;
+  }
+) => {
+  code: string;
+  map: SourceMap;
+} | null;
+
+export function createRollupDebugIdInjectionHooks(): {
+  renderChunk: RenderChunkHook;
+} {
   return {
     renderChunk(code: string, chunk: { fileName: string }) {
       if (
@@ -311,7 +321,9 @@ export function createRollupDebugIdInjectionHooks() {
   };
 }
 
-export function createRollupModuleMetadataInjectionHooks(injectionCode: string) {
+export function createRollupModuleMetadataInjectionHooks(injectionCode: string): {
+  renderChunk: RenderChunkHook;
+} {
   return {
     renderChunk(code: string, chunk: { fileName: string }) {
       if (
@@ -349,7 +361,12 @@ export function createRollupDebugIdUploadHooks(
   upload: (buildArtifacts: string[]) => Promise<void>,
   _logger: Logger,
   createDependencyOnBuildArtifacts: () => () => void
-) {
+): {
+  writeBundle: (
+    outputOptions: { dir?: string; file?: string },
+    bundle: { [fileName: string]: unknown }
+  ) => Promise<void>;
+} {
   const freeGlobalDependencyOnDebugIdSourcemapArtifacts = createDependencyOnBuildArtifacts();
   return {
     async writeBundle(
@@ -390,7 +407,9 @@ export function createRollupDebugIdUploadHooks(
   };
 }
 
-export function createComponentNameAnnotateHooks(ignoredComponents?: string[]) {
+export function createComponentNameAnnotateHooks(ignoredComponents?: string[]): {
+  transform: UnpluginOptions["transform"];
+} {
   type ParserPlugins = NonNullable<
     NonNullable<Parameters<typeof transformAsync>[1]>["parserOpts"]
   >["plugins"];

packages/bundler-plugin-core/src/sentry/telemetry.ts

@@ -64,7 +64,7 @@ export function setTelemetryDataOnScope(
   options: NormalizedOptions,
   scope: Scope,
   buildTool: string
-) {
+): void {
   const { org, project, release, errorHandler, sourcemaps, reactComponentAnnotation } = options;
 
   scope.setTag("upload-legacy-sourcemaps", !!release.uploadLegacySourcemaps);
@@ -161,7 +161,7 @@ export async function allowedToSendTelemetry(options: NormalizedOptions): Promis
 /**
  * Flushing the SDK client can fail. We never want to crash the plugin because of telemetry.
  */
-export async function safeFlushTelemetry(sentryClient: Client) {
+export async function safeFlushTelemetry(sentryClient: Client): Promise<void> {
   try {
     await sentryClient.flush(2000);
   } catch {

packages/bundler-plugin-core/src/sentry/transports.ts

@@ -93,7 +93,7 @@ function createRequestExecutor(options: BaseTransportOptions): TransportRequestE
 /**
  * Creates a Transport that uses native the native 'http' and 'https' modules to send events to Sentry.
  */
-function makeNodeTransport(options: BaseTransportOptions) {
+function makeNodeTransport(options: BaseTransportOptions): Transport {
   const requestExecutor = createRequestExecutor(options);
   return createTransport(options, requestExecutor);
 }

packages/bundler-plugin-core/src/utils.ts

@@ -311,7 +311,7 @@ export function generateGlobalInjectorCode({
 }: {
   release: string;
   injectBuildInformation: boolean;
-}) {
+}): string {
   // The code below is mostly ternary operators because it saves bundle size.
   // The checks are to support as many environments as possible. (Node.js, Browser, webworkers, etc.)
   let code = `{
@@ -341,7 +341,7 @@ export function generateGlobalInjectorCode({
 }
 
 // eslint-disable-next-line @typescript-eslint/no-explicit-any
-export function generateModuleMetadataInjectorCode(metadata: any) {
+export function generateModuleMetadataInjectorCode(metadata: any): string {
   // The code below is mostly ternary operators because it saves bundle size.
   // The checks are to support as many environments as possible. (Node.js, Browser, webworkers, etc.)
   // We are merging the metadata objects in case modules are bundled twice with the plugin
@@ -369,7 +369,11 @@ export function generateModuleMetadataInjectorCode(metadata: any) {
 }`;
 }
 
-function getBuildInformation() {
+export function getBuildInformation(): {
+  deps: string[];
+  depsVersions: Record<string, number>;
+  nodeVersion: number | undefined;
+} {
   const packageJson = getPackageJson();
 
   const { deps, depsVersions } = packageJson
@@ -413,7 +417,7 @@ export function replaceBooleanFlagsInCode(
 }
 
 // https://turbo.build/repo/docs/reference/system-environment-variables#environment-variables-in-tasks
-export function getTurborepoEnvPassthroughWarning(envVarName: string) {
+export function getTurborepoEnvPassthroughWarning(envVarName: string): string {
   return process.env["TURBO_HASH"]
     ? `\nYou seem to be using Turborepo, did you forget to put ${envVarName} in \`passThroughEnv\`? https://turbo.build/repo/docs/reference/configuration#passthroughenv`
     : "";

packages/bundler-plugin-core/test/utils.test.ts

@@ -10,7 +10,7 @@ import path from "node:path";
 
 type PackageJson = Record<string, unknown>;
 
-function getCwdFor(dirName: string) {
+function getCwdFor(dirName: string): string {
   return path.resolve(__dirname + dirName);
 }
 

packages/e2e-tests/.eslintrc.js

@@ -25,5 +25,6 @@ module.exports = {
   },
   rules: {
     "no-console": "off",
+    "@typescript-eslint/explicit-function-return-type": "off",
   },
 };

packages/eslint-configs/base.js

@@ -15,5 +15,9 @@ module.exports = {
       { argsIgnorePattern: "^_", caughtErrorsIgnorePattern: "^_" },
     ],
     "no-undef": "error", // https://github.com/typescript-eslint/typescript-eslint/issues/4580#issuecomment-1047144015
+    // Although for most codebases inferencing the return type is fine, we explicitly ask to annotate
+    // all functions with a return type. This is so that intent is as clear as possible as well as to
+    // avoid accidental breaking changes.
+    "@typescript-eslint/explicit-function-return-type": ["error", { allowExpressions: true }],
   },
 };