Skip to content

Commit

Permalink
[compiler][eslint] Use logger callback instead of exceptions to repor…
Browse files Browse the repository at this point in the history
…t eslint diagnostics

---
* panicThreshold: `all_errors` -> `none`
* inject an error logger through compiler config (instead of using exceptions)

We currently report at most one lint warning per file, this lets us exhaustively report all available ones (see new
test fixture for example)

ghstack-source-id: 5299315574d11929efc39ee8f6033e3035d1e378
Pull Request resolved: #30336
  • Loading branch information
mofeiZ committed Jul 15, 2024
1 parent 6cca9c3 commit e307a49
Show file tree
Hide file tree
Showing 2 changed files with 60 additions and 24 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,36 @@ const tests: CompilerTestCases = {
},
],
},
{
name: "Multiple diagnostics are surfaced",
options: [
{
reportableLevels: new Set([
ErrorSeverity.Todo,
ErrorSeverity.InvalidReact,
]),
},
],
code: normalizeIndent`
function Foo(x) {
var y = 1;
return <div>{y * x}</div>;
}
function Bar(props) {
props.a.b = 2;
return <div>{props.c}</div>
}`,
errors: [
{
message:
"(BuildHIR::lowerStatement) Handle var kinds in VariableDeclaration",
},
{
message:
"Mutating component props or hook arguments is not allowed. Consider using a local variable instead",
},
],
},
],
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,9 @@ import BabelPluginReactCompiler, {
ErrorSeverity,
parsePluginOptions,
validateEnvironmentConfig,
type CompilerError,
type PluginOptions,
} from "babel-plugin-react-compiler/src";
import { Logger } from "babel-plugin-react-compiler/src/Entrypoint";
import type { Rule } from "eslint";
import * as HermesParser from "hermes-parser";

Expand All @@ -29,10 +29,6 @@ function assertExhaustive(_: never, errorMsg: string): never {
throw new Error(errorMsg);
}

function isReactCompilerError(err: Error): err is CompilerError {
return err.name === "ReactCompilerError";
}

const DEFAULT_REPORTABLE_LEVELS = new Set([
ErrorSeverity.InvalidReact,
ErrorSeverity.InvalidJS,
Expand Down Expand Up @@ -105,7 +101,7 @@ function makeSuggestions(
const COMPILER_OPTIONS: Partial<PluginOptions> = {
noEmit: true,
compilationMode: "infer",
panicThreshold: "all_errors",
panicThreshold: "none",
};

const rule: Rule.RuleModule = {
Expand Down Expand Up @@ -137,6 +133,33 @@ const rule: Rule.RuleModule = {
...parsePluginOptions(userOpts),
...COMPILER_OPTIONS,
};
const userLogger: Logger | null = options.logger;
options.logger = {
logEvent: (filename, event): void => {
userLogger?.logEvent(filename, event);
if (event.kind === "CompileError") {
const detail = event.detail;
if (!isReportableDiagnostic(detail)) {
return;
}
if (hasFlowSuppression(detail.loc, "react-rule-hook")) {
// If Flow already caught this error, we don't need to report it again.
return;
}
const loc =
detail.loc == null || typeof detail.loc == "symbol"
? event.fnLoc
: detail.loc;
if (loc != null) {
context.report({
message: detail.reason,
loc,
suggest: makeSuggestions(detail),
});
}
}
},
};

try {
options.environment = validateEnvironmentConfig(
Expand Down Expand Up @@ -206,24 +229,7 @@ const rule: Rule.RuleModule = {
babelrc: false,
});
} catch (err) {
if (isReactCompilerError(err) && Array.isArray(err.details)) {
for (const detail of err.details) {
if (!isReportableDiagnostic(detail)) {
continue;
}
if (hasFlowSuppression(detail.loc, "react-rule-hook")) {
// If Flow already caught this error, we don't need to report it again.
continue;
}
context.report({
message: detail.reason,
loc: detail.loc,
suggest: makeSuggestions(detail),
});
}
} else {
options.logger?.logEvent("", err);
}
/* errors handled by injected logger */
}
}
return {};
Expand Down

0 comments on commit e307a49

Please sign in to comment.