Skip to content

Commit

Permalink
fix(fe): don't report React-specific JSX errors in non-React code
Browse files Browse the repository at this point in the history
Preact uses JSX but has different rules for attributes. Disable our
React-specific rules if a React import is not detected.
  • Loading branch information
strager committed Jan 5, 2024
1 parent a985342 commit cf29858
Show file tree
Hide file tree
Showing 9 changed files with 179 additions and 20 deletions.
5 changes: 5 additions & 0 deletions docs/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,9 @@ Semantic Versioning.

* quick-lint-js's tracing no longer crashes with an assertion failure when
setting its thread name on FreeBSD.
* React-specific JSX diagnostics, such as [E0193][] ("misspelled React
attribute; write 'className' instead"), are now only reported when 'react' is
imported. This fixes false warnings in Preact code. ([#1152][])

## 3.0.0 (2024-01-01)

Expand Down Expand Up @@ -1376,6 +1379,8 @@ Beta release.
[toastin0]: https://github.com/toastin0
[wagner riffel]: https://github.com/wgrr

[#1152]: https://github.com/quick-lint/quick-lint-js/issues/1152

[E0001]: https://quick-lint-js.com/errors/E0001/
[E0003]: https://quick-lint-js.com/errors/E0003/
[E0013]: https://quick-lint-js.com/errors/E0013/
Expand Down
4 changes: 4 additions & 0 deletions docs/errors/E0191.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ In HTML, attributes are case-insensitive; `onclick` is the same as `onClick` and
attribute (starting with `on`) to be all lower-case:

```javascript-jsx
import React from "react";
function TodoEntry({addTodo, changePendingTodo}) {
return <form onsubmit={addTodo}>
<input onchange={changePendingTodo} />
Expand All @@ -17,6 +19,8 @@ To fix this error, fix the capitalization by writing the attribute in
lowerCamelCase:

```javascript-jsx
import React from "react";
function TodoEntry({addTodo, changePendingTodo}) {
return <form onSubmit={addTodo}>
<input onChange={changePendingTodo} />
Expand Down
4 changes: 4 additions & 0 deletions docs/errors/E0192.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ In HTML, attributes are case-insensitive; `colspan` is the same as `colSpan` and
attribute for a built-in element to have the wrong capitalization:

```javascript-jsx
import React from "react";
function Header({columns}) {
return <tr>
<th colspan="2">Name</th>
Expand All @@ -17,6 +19,8 @@ function Header({columns}) {
To fix this error, fix the capitalization of the attribute:

```javascript-jsx
import React from "react";
function Header({columns}) {
return <tr>
<th colSpan="2">Name</th>
Expand Down
4 changes: 4 additions & 0 deletions docs/errors/E0193.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ React has a different name for some attributes than HTML. It is a mistake to
write the HTML attribute instead of the React attribute:

```javascript-jsx
import React from "react";
function Title({page}) {
return <h1 class="title">
<a href={page.url} class="page-link">
Expand All @@ -16,6 +18,8 @@ function Title({page}) {
To fix this error, write the name of the attribute understood by React:

```javascript-jsx
import React from "react";
function Title({page}) {
return <h1 className="title">
<a href={page.url} className="page-link">
Expand Down
2 changes: 1 addition & 1 deletion src/quick-lint-js/fe/parse-expression.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3851,7 +3851,7 @@ Expression* Parser::parse_jsx_element_or_fragment(Parse_Visitor_Base& v,
this->lexer_.skip_in_jsx();
}
if (is_intrinsic && !has_namespace && !tag_namespace) {
this->check_jsx_attribute(attribute);
this->jsx_intrinsic_attributes_.emplace_back(attribute);
}
if (this->peek().type == Token_Type::equal) {
this->lexer_.skip_in_jsx();
Expand Down
16 changes: 16 additions & 0 deletions src/quick-lint-js/fe/parse-statement.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ void Parser::parse_and_visit_module(Parse_Visitor_Base &v) {
}
}
}
this->check_all_jsx_attributes();
v.visit_end_of_module();
}

Expand Down Expand Up @@ -4695,6 +4696,7 @@ void Parser::parse_and_visit_import(
// import "foo";
case Token_Type::string:
// Do not set is_current_typescript_namespace_non_empty_.
this->visited_module_import(this->peek());
this->skip();
this->consume_semicolon_after_statement();
return;
Expand Down Expand Up @@ -4853,6 +4855,7 @@ void Parser::parse_and_visit_import(

this->skip();
QLJS_PARSER_UNIMPLEMENTED_IF_NOT_TOKEN(Token_Type::string);
this->visited_module_import(this->peek());
this->skip();
QLJS_PARSER_UNIMPLEMENTED_IF_NOT_TOKEN(Token_Type::right_paren);
this->skip();
Expand Down Expand Up @@ -4918,6 +4921,7 @@ void Parser::parse_and_visit_import(
.declare_keyword = *declare_context.declare_namespace_declare_keyword,
});
}
this->visited_module_import(this->peek());
this->skip();
break;

Expand Down Expand Up @@ -5313,6 +5317,18 @@ void Parser::parse_and_visit_named_exports(
this->skip();
}

void Parser::visited_module_import(const Token &module_name) {
QLJS_ASSERT(module_name.type == Token_Type::string);
// TODO(#1159): Write a proper routine to decode string literals.
String8_View module_name_unescaped =
make_string_view(module_name.begin + 1, module_name.end - 1);
if (module_name_unescaped == u8"react"_sv ||
module_name_unescaped == u8"react-dom"_sv ||
starts_with(module_name_unescaped, u8"react-dom/"_sv)) {
this->imported_react_ = true;
}
}

void Parser::parse_and_visit_variable_declaration_statement(
Parse_Visitor_Base &v,
bool is_top_level_typescript_definition_without_declare_or_export) {
Expand Down
21 changes: 21 additions & 0 deletions src/quick-lint-js/fe/parse.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,27 @@ Expression* Parser::build_expression(Binary_Expression_Builder& builder) {
}
}

void Parser::check_all_jsx_attributes() {
switch (this->options_.jsx_mode) {
case Parser_JSX_Mode::none:
break;

case Parser_JSX_Mode::auto_detect:
if (this->imported_react_) {
goto react;
}
break;

react:
case Parser_JSX_Mode::react:
this->jsx_intrinsic_attributes_.for_each(
[&](const Identifier& attribute_name) -> void {
this->check_jsx_attribute(attribute_name);
});
break;
}
}

QLJS_WARNING_PUSH
QLJS_WARNING_IGNORE_GCC("-Wnull-dereference")
void Parser::check_jsx_attribute(const Identifier& attribute_name) {
Expand Down
33 changes: 33 additions & 0 deletions src/quick-lint-js/fe/parse.h
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
#include <optional>
#include <quick-lint-js/assert.h>
#include <quick-lint-js/container/hash-map.h>
#include <quick-lint-js/container/linked-vector.h>
#include <quick-lint-js/container/padded-string.h>
#include <quick-lint-js/diag/diag-reporter.h>
#include <quick-lint-js/diag/diagnostic-types.h>
Expand Down Expand Up @@ -55,11 +56,30 @@ enum class Parser_Top_Level_Await_Mode {
await_operator,
};

// How to interpret props for builtins such as <button onclick> and <span
// class>.
enum class Parser_JSX_Mode {
// Detect the JSX mode based on heuristics.
auto_detect = 0,

// Enforce no rules.
none,

// Use React's rules:
// * camelCase for event handler attributes starting with 'on'
// * certain camelCase attributes such as 'colSpan'
// * 'class' is named 'className'
react,
};

// TODO(#465): Accept parser options from quick-lint-js.config or CLI options.
struct Parser_Options {
Parser_Top_Level_Await_Mode top_level_await_mode =
Parser_Top_Level_Await_Mode::auto_detect;

// jsx_mode does not affect whether JSX is parsed or not. See this->jsx.
Parser_JSX_Mode jsx_mode = Parser_JSX_Mode::auto_detect;

// If true, parse JSX language extensions: https://facebook.github.io/jsx/
bool jsx = false;

Expand Down Expand Up @@ -608,6 +628,9 @@ class Parser {
void parse_and_visit_named_exports_for_typescript_type_only_import(
Parse_Visitor_Base &v, Source_Code_Span type_keyword);

// Precondition: module_name.type == Token_Type::string;
void visited_module_import(const Token &module_name);

// If set, refers to the first `export default` statement in this module. A
// module cannot contain more than one `export default`.
std::optional<Source_Code_Span>
Expand Down Expand Up @@ -887,6 +910,7 @@ class Parser {
Expression *parse_jsx_element_or_fragment(Parse_Visitor_Base &,
Identifier *tag,
const Char8 *less_begin);
void check_all_jsx_attributes();
void check_jsx_attribute(const Identifier &attribute_name);
Expression *parse_typescript_generic_arrow_expression(Parse_Visitor_Base &,
Precedence);
Expand Down Expand Up @@ -1092,6 +1116,15 @@ class Parser {
// variables) so that memory can be released in case we call setjmp.
Buffering_Visitor_Stack buffering_visitor_stack_;

// All parsed JSX attributes for intrinsic elements.
//
// Depending on Parser_JSX_Mode, diagnostics may or may not be reported for
// these attributes at the end of a module.
Linked_Vector<Identifier> jsx_intrinsic_attributes_{new_delete_resource()};

// Heuristic. True if React.js was imported.
bool imported_react_ = false;

bool in_top_level_ = true;
bool in_async_function_ = false;
bool in_generator_function_ = false;
Expand Down
Loading

0 comments on commit cf29858

Please sign in to comment.