oxc-project / oxc Goto Github PK

The conformance task now snapshots all diagnostics:

• test262.snap
• babel.snap
• typescript.snap

What you can do is review these diagnostics and improve them, you can checkout babel or tsc for reference, by using https://astexplorer.net/

To begin with, it should be relative easy to fix some of the Unexpected token errors.

This umbrella lays out all the tasks for the linter.

The decisions are made off #38.

• #44
• #46
• #51
• Parse eslint rule configurations

Problem
Printing lint errors slows down exponentially.
Slowness is significant once the threshold of a few hundred diagnostics is crossed.

Repro

1. Download babylon.max.js (arbitrary example)
2. Build release
3. Run ./oxc_cli.exe lint --max-warnings 1 babylon.max.js - very fast E2E time (to give an idea of parsing time)
4. Run ./oxc_cli.exe lint --max-warnings 10 babylon.max.js - E2E time barely increased.
5. Run ./oxc_cli.exe lint --max-warnings 100 babylon.max.js - E2E time increased much more in relative terms. On my machine, it's ~60x, where the number of printed messages only grew 10x (90 vs 9).
6. Run ./oxc_cli.exe lint --max-warnings 500 babylon.max.js - time 20x,
7. You get the point..

Initial findings
I thought it would be related to writing to stdout, but that seems to be done correctly already:

 let mut buf_writer = BufWriter::new(std::io::stdout());
        let mut number_of_warnings = 0;
        let mut number_of_diagnostics = 0;

        while let Ok(diagnostic) = rx_error.recv() {
            number_of_diagnostics += 1;

            if diagnostic.severity() == Some(Severity::Warning) {
                number_of_warnings += 1;
                // The --quiet flag follows ESLint's --quiet behavior as documented here: https://eslint.org/docs/latest/use/command-line-interface#--quiet
                // Note that it does not disable ALL diagnostics, only Warning diagnostics
                if self.cli_options.quiet {
                    continue;
                }

                if let Some(max_warnings) = self.cli_options.max_warnings {
                    if number_of_warnings > max_warnings {
                        continue;
                    }
                }
            }

            buf_writer
                .write_all(format!("{diagnostic:?}").as_bytes())
                .expect("Failed to write diagnostic.");
        }

One thing we can improve is take the lock on stdout. However, that doesn't solve the issue.

Profiling shows all of the time spent in miette:

The culprit isn't immediately obvious.
I haven't dug into miette code yet. It seems unlikely that it would have such a big performance issue, with no smoke (I couldn't find any issues in the repo related to performance).

The exponential nature of the slowness hints towards some vector or other struct's memory not being cleared/reused efficiently.

This blocks #118

The current prototype always exit with 0 because I haven't implemented anything related to exit code.

In this good first issue, you will learn about the std::process::Termination trait and the std::process::ExitCode struct.

In oxc_cli, you may define a wrapper enum for all the possible exit states

#[derive(Debug)]
pub enum CliRunResult {
    None,
    PathNotFound {
        path: PathBuf,
    },
    LintResult {
        ... bunch of stats and stuff
    },
}

and nice messages inside the report method.

Finally inside main.rs, you can return the run result with std::process::Termination implemented on it.

fn main() -> CliRunResult {}

Have fun!

We have code like these:

    #[serde(skip_serializing_if = "Option::is_none")]
    pub decorators: Option<Vec<'a, Decorator<'a>>>,

I was in the mentality of None = JavaScript and didn't think more deeply.

This should just be

#[serde(skip_serializing_if = "Vec::is_empty")]

From the v8 scanner blog, I still see low hanging fruits, let's play with these and make our lexer even faster!

Cargo fix does this, which is really nice.

See:

• https://github.com/rust-lang/cargo/blob/master/src/cargo/ops/fix.rs#L136
• https://github.com/rust-lang/cargo/blob/master/tests/testsuite/fix.rs#L710-L816

After I wrap up the work on #57 it'd be great to try to implement some lint rules, do you have any particular rules in mind that you'd like implemented? 🙂

for test of test262/test/language/asi/S7.9_A6.2_T10.js

for(
    false
;) {
  break;
}

oxc now only prints Unexpected token for expressions missing semicolon.

  × Unexpected token
    ╭─[language/asi/S7.9_A6.2_T1.js:19:1]
 19 │ for(;
 20 │ ) {
    · ─
 21 │   break;
    ╰────

It would be better for the diagnostics to print ';' expected since we are in a for loop. An insertion of semi will simply help complete the for statement. See diagnostics of tsc

Implement the ESLint constructor-super rule

configuration comments, a.k.a suspension comments, eslint-disable-next

https://eslint.org/docs/latest/use/configure/rules#disabling-rules

background: #6

Here's what I have in mind, please correct me if you don't think it is not going to work:

To get configuration comments to work, we need to:

1. inside the lexer, save all the eslint configuration comments and the span they apply to
2. when a diagnostic is emitted, find if it overlaps with the comment ranges saved from above
3. report all unused comments (the reportUnusedDisableDirectives option)

The first step will be the most difficult part. I don't know if it is even possible, but it'll be much more efficient than eslint because we don't need to track line numbers.

Motivation: #79

https://eslint.org/docs/latest/rules/no-constant-binary-expression

I'll implement this one.

          @Boshen is there a reason `start` and `end` on `Span` are `u32`?

Originally posted by @shannonrothe in #101 (comment)

https://twitter.com/zakarum4/status/1630669810110545921

https://github.com/zakarumych/blink-alloc

cc @zakarumych

It would be nice to see the total execution time when linting is done.

This file is getting huge and it is just repetitive boilerplate. It would be nice to use a proc_macro to generate these.

The file should end up with something like

declare_all_lints! {
    no_debugger,
    typescript::no_empty_interface,
    deepscan::uninvoked_array_callback
}

The identifier deepscan::uninvoked_array_callback should be a syn::Path I think.

It is expanded into

mod deepscan {
  pub mod uninvoked_array_callback;
}

For example

https://github.com/Boshen/oxc/blob/4829b85c2d89cfc8241e8542e49f4f9a8f8d36ae/crates/oxc_linter/src/rules.rs#L5-L7

has this directory structure

The Token struct is currently 80 bytes carrying a 56 byte TokenValue. This should be reduced.

Let's study this and implement it https://github.com/eslint/eslint/blob/main/lib/linter/source-code-fixer.js

Clippy defines the following rule categories for lint rules

I haven't thought much about the to categorize JavaScript / TypeScript rules, but we should at least define a correctness and nursery rule so we can ship and merge unfinished rules at the same time.

This should be an easy task where we copy how Clippy does this.

We will need to define correctness and nursery.

Rules have metadata we want to access to, but it is tedious to implement a RuleMeta trait on every lint rule.

In this good first issue, we will follow the more difficult paths of following rust-clippy, e.g.

https://github.com/rust-lang/rust-clippy/blob/e1ac1332b668f584919aba86ad3e81f267251e9b/clippy_lints/src/almost_complete_range.rs#L11-L31

So we need to:

• add a declare_oxc_lint proc_macro crate
• define a RuleMeta trait
• hook up the NoDebugger Rule with the proc macro
• add some tests for this, maybe in the same file where the RuleMeta trait is.

The first prototype should probably just contain a name and a documentation. Other metadata can be added later.

See

• Rome https://github.com/rome/tools/blob/5c60450e016318c761590e883fa33d780491d08c/crates/rome_analyze/src/rule.rs#L83
• Ruff https://github.com/charliermarsh/ruff/blob/3179fc110d815d9b5cb9a3179bad6e1246a5bbf8/crates/ruff_macros/src/lib.rs#L29-L34
• Clippy https://github.com/rust-lang/rust-clippy/blob/master/declare_clippy_lint/src/lib.rs https://github.com/rust-lang/rust-clippy/blob/e1ac1332b668f584919aba86ad3e81f267251e9b/clippy_lints/src/almost_complete_range.rs#L11-L31

Drop a comment if you wish to tackle this, so I can assign this issue to you.

For our first good release, we want to have some correctness rules.

Correctness rules (defined by clippy) are code that is outright wrong or useless.

Some of these are from deepscan or eslint-plugin-import.

Only ASTs rules can implemented right now, but I'll be adding scopes and symbol resolution soon.

Rules that require CFG (Control flow graph) cannot be implemented right now.

Deepscan:

• bad-array-method-on-arguments
• bad-assign-to-proto
• bad-bitwise-operator
• bad-comparison-sequence
• bad-min-max-func
• bad-month-arg
• bad-remove-event-listener
• bad-string-case-comparison
• number-arg-out-of-range
• https://deepscan.io/docs/rules/bad-replace-all-arg
• https://deepscan.io/docs/rules/bad-match-all-arg

eslint-plugin-import:

• import/export
• import/named
• import/no-absolute-path
• import/no-named-as-default-member
• import/no-self-import

ESLint:

• array-callback-return
• #127
• #136
• getter-return
• missing-throw
• no-async-promise-executor #180
• no-caller #178
• no-class-assign #204
• no-compare-neg-zero #185
• no-const-assign #199
• no-constant-binary-expression #118
• no-constant-condition
• no-debugger
• no-dupe-class-members #195
• no-dupe-else-if
• no-dupe-keys #188
• no-duplicate-case #179
• no-eval
• no-func-assign
• no-implied-eval
• no-import-assign
• no-irregular-whitespace
• no-new-symbol #201
• no-obj-calls
• no-self-compare #152
• no-setter-return
• no-shadow-restricted-names
• no-this-before-super
• no-unreachable
• no-unsafe-negation #186
• no-unused-labels
• require-yield
• #112
• use-isnan #161
• valid-typeof #202

Syntax error

The rules in here don't need to be implemented since they are syntax errors.

• no-with (strict mode error)
• no-dupe-args #181
• no-octal

Description

Typescript modifiers, e.g., abstract, public, declare, can appear in arbitrary order and they can modify any kind of declarations, not only class. For example, abstract const a = 1; can produce a valid AST and the semantic analyzer would reject the code because abstract can only modify class declaration.

Expected Behavior

oxc_parser should be able to produce valid AST for these kinds of constructs

Are you planning on fix it.

Yes

We should create a oxc_wasm crate and export the parser so we can build a playground.

See https://github.com/rome/tools/tree/main/crates/rome_wasm

#23 (comment)

I have found that to_bitmask on ARM NEON can be cause parsing to be slower compared to a scalar version. My block comment parser only uses SIMD on x86, not ARM. Do you run your benchmarks on ARM NEON or just Intel SSE?

On x86_64, to_bitmask is one instruction. On AArch64, there is no instruction and to_bitmask needs to be emulated. See: https://godbolt.org/z/531oEo5d1 It looks like the Rust compiler currently does a terrible job on AArch64. You can definitely do better by hand, but it still might not be faster than the scalar version.

It seems I need a way to verify the code is correct.

Basically a parse -> print -> parse routine will sufficiently indicate if parsing is correct.

#26 (comment)

You can guarantee that remainder is always empty or unimportant (thus you don't need to check for it) if you add enough zeros at the end of bytes. I do this in quick-lint-js when I load the file from the filesystem, so everything can safely assume that the extra bytes are present. (I have a dedicated type for this called padded_string_view which doesn't allow breaking this invariant.)

I wonder if it's possible for us to implement nested code fixes. For example, the following expression:
1 & 2 ^ 3 | 4
should be changed to ((1 & 2) ^ 3) | 4, which involves first changing the span 1 & 2 to be (1 & 2) and then changing (1 & 2) ^ 3 to be ((1 & 2) ^ 3).

I noticed our current fixer seems to assume its fix spans are disjoint intervals. Could this be done with our current approach, i.e., manipulating source text directly rather than changing the AST?

@YangchenYe323 @YoniFeng Let's continue our adventure after the video https://www.youtube.com/watch?v=DMQ_HcNSOAI

@YangchenYe323 Can you draft a baseline line PR, and let's walk up the performance ladder. We don't need to go all the way.

I need to setup the fuzzer for the parser :-)

See https://twitter.com/jarredsumner/status/1485397686224257027 for baselines

From Twitter:

• MiniJinja author said avoiding ? improved both the compile time and the runtime performances.
• dtolnay said they don't use ? operators in serde and serde-json "because it is too slow".
• serde has their own try! macro that avoids type conversion https://github.com/rust-lang/rust/issues/37939
• mitsuhiko says its still not fix

u32 is enough for spans.

https://github.com/Boshen/oxc/blob/b6912513ba93ebb01ebc8882598951e26a79ea6c/crates/oxc_ast/src/node.rs#L11-L18

This is currently using usize because miette only accepts Range in its miette::Diagnostic macro.

https://github.com/Boshen/oxc/blob/b6912513ba93ebb01ebc8882598951e26a79ea6c/crates/oxc_diagnostics/src/lib.rs#L29-L33

There should be a workaround but I haven't investigated in detail yet.

This is going to be fun!

We will be porting the no-array-constructor rule for JavaScript and TypeScript at the same time:

• https://eslint.org/docs/latest/rules/no-array-constructor
• https://typescript-eslint.io/rules/no-array-constructor

The goal of this task is to:

• iron out the details for implementing a rule
• draft a documentation for how to implement a rule
• nice to have - create a script to transform tests to our Rust, because it is going to be very time consuming to copy, paste and modify the tests

Your feedbacks will shape the future of this project.

I have also setup a discord channel for quick communications.

Drop a comment if you wish to try this, so I can assign this issue to you.

https://eslint.org/docs/latest/use/command-line-interface#run-the-cli

npx eslint [options] [file|dir|glob]*

Please note that when passing a glob as a parameter, it is expanded by your shell. The results of the expansion can vary depending on your shell, and its configuration. If you want to use node glob syntax, you have to quote your parameter

Make a comment if you wish to tackle this, so I can assign this issue to you.

I want to get the ball rolling for releasing binaries. This is the initial step for our most viable product.

• #80
• #89

This blocks #118

Relevant: https://github.com/swc-project/swc/issues/7019

I'll take a look at the AST and see if there are any further improvements we can made, I already boxed all the enums but there may be some low hanging fruits.

Commands

cargo install top-type-sizes
RUSTFLAGS=-Zprint-type-sizes cargo build --release -p oxc_ast -j 1 > type-sizes.txt
top-type-sizes -w -f ast:: < type-sizes.txt > output.txt

</details>

We are currently constructing a parent pointing tree using the indextree crate, but this is probably an overkill both cpu and memory wise.

The only functionality we need are:

• find a node's parent node
• iterate through a node's ancestor nodes

For performance, I wonder if using bumpalo will be better than a Vec by using the same allocator for the AST.

The problem with clap is that it's binary size is huge! And it compiles really slowly. You cna see the bloat here: https://github.com/Boshen/oxc/actions/runs/4336082139

Someone suggested google/argh, let's try it.

https://eslint.org/docs/latest/use/command-line-interface#ignore-files

It would be the best if you submit a PR for each command.

Make a comment if you wish to tackle this, so I can assign this issue to you.

This is caught by the fuzzer https://github.com/Boshen/oxc/actions/runs/4395420223/jobs/7697210796

To replicate:

Create a test.tsx with the following string:

1<(V=82<<t-j0<(V=$<LBI<(V=ut<I<(V=$<LBI<(V=uIV=82<<t-j0<(V=$<LBI<(V=ut<I<(V=$<LBI<(V<II>

run cargo run -p oxc_parser --example parser -- test.tsx

and it will run forever.

I'm gussing this is the result of parsing the ambiguous < token, where TypeScript needs to lookahead and rewind forever.

https://eslint.org/docs/latest/use/command-line-interface#handle-warnings

Make a comment if you wish to tackle this, so I can assign this issue to you.

• --quiet #72
• --max-warnings TODO

We need to check git status before applying any fixes otherwise we'll blow up people's code.

Clippy does this if you haven't noticed ;-)

This check has to live inside the cli for now because we don't have a file system abstraction. All the other components will be wasmed eventually.

I see there are some demand for reading jsdoc information, let's do this in two steps:

1. parse jsdoc into structured data, this should be lazy.
2. save them into trivias and have them accessible. I haven't figured out the best way to do this, but see #6 for disucssion.

Note: This is for parsing jsdoc comments to structured data. This is not about wiring up all the jsdoc comments into a document tree (which is the main purpose for jsdoc).

To parse jsdoc comments, we need to:

1. add a new routine to parse jsdoc in oxc_parser, this should be a separate routine so we can parse jsdoc lazily
2. add the jsdoc AST to oxc_ast
3. setup jsdoc tests (I'm unsure on how to do this properly yet).

References:

• https://jsdoc.app/
• tests
  • https://github.com/jsdoc/jsdoc/tree/main/packages/jsdoc/test
  • typescript conformance tests: https://github.com/microsoft/TypeScript/tree/main/tests/cases/conformance/jsdoc

Motivation: #79

This is from deepscan, https://deepscan.io/docs/rules/uninvoked-array-callback

TypeScript doesn't implement this.

I'll implement this one.

Sometime ago I drafted a version to use SIMD for skipping whitespaces:

In this good first issue, you should be able to replicate the code from above, and add it to the lexer.

You should refactor the code above into a function and call the function just above the while loop:

https://github.com/Boshen/oxc/blob/93993978de969589f1fb91a689594a5e5c4b8f02/crates/oxc_parser/src/lexer/mod.rs#L320-L330

You will also need to add #![feature(portable_simd)] to the top of crates/oxc_parser/src/lib.rs

To the run parser, you can run the example binary:

https://github.com/Boshen/oxc/blob/3daf5bcea95f6fe2c88a7956948f4cb22711f99e/crates/oxc_parser/examples/simple.rs#L7-L10

Background reading:

• https://rapidjson.org/md_doc_internals.html#SkipwhitespaceWithSIMD
• https://github.com/Tencent/rapidjson/blob/bdc49ad80a3cff89a25839dcdcd6a68b457ec9f1/include/rapidjson/reader.h#L258-L517
• https://lemire.me/blog/2017/01/20/how-quickly-can-you-remove-spaces-from-a-string

Compact string currently holds a capacity, which isn't needed for our usecase.

Sorry if this may come out as rude. But I'm not a rust developer, I saw oxc on github explore feed, and wanted to try it out (you know, let it "oxidise" my javascript), but the README doesn't have those instructions.

Usually, one would like to include "Installation" and "Usage" headings to help new users learn how to use it...