Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(semantic): parse jsdoc comments #205

Merged
merged 4 commits into from
Apr 12, 2023
Merged

feat(semantic): parse jsdoc comments #205

merged 4 commits into from
Apr 12, 2023

Conversation

shannonrothe
Copy link
Contributor

@shannonrothe shannonrothe commented Mar 24, 2023

@Boshen I took a very naive first pass and it's not completely functional. I'd love to see your ideas around using OnceCell and how we can properly attribute a given comment span to a node span.

I attempted the lazy OnceCell approach in this commit (9cefee4), but OnceCell is not Copy so it doesn't compile

@shannonrothe shannonrothe changed the title feat(semantic) parse jsdoc comments feat(semantic): parse jsdoc comments Mar 24, 2023
tasks/jsdoc/Cargo.toml Outdated Show resolved Hide resolved
@github-actions
Copy link
Contributor

github-actions bot commented Mar 24, 2023

Benchmark Results

Linux

group                      main                                   pr
-----                      ----                                   --
parser/babylon.max.js      1.02    134.5±5.56ms    76.8 MB/sec    1.00    132.1±4.25ms    78.1 MB/sec
parser/d3.js               1.02     16.1±0.66ms    34.0 MB/sec    1.00     15.8±0.43ms    34.7 MB/sec
parser/lodash.js           1.02      4.5±0.16ms   114.2 MB/sec    1.00      4.4±0.18ms   116.9 MB/sec
parser/pdf.js              1.00      8.9±0.34ms    45.2 MB/sec    1.03      9.2±0.58ms    43.9 MB/sec
parser/typescript.js       1.04    136.6±7.40ms    70.4 MB/sec    1.00    131.6±4.68ms    73.1 MB/sec
semantic/babylon.max.js    1.00    276.6±5.65ms    37.3 MB/sec    1.01    279.6±7.37ms    36.9 MB/sec
semantic/d3.js             1.00     31.7±2.01ms    17.3 MB/sec    1.11     35.1±2.36ms    15.6 MB/sec
semantic/lodash.js         1.00      6.7±0.37ms    77.1 MB/sec    1.08      7.2±0.86ms    71.5 MB/sec
semantic/pdf.js            1.03     13.6±1.24ms    29.6 MB/sec    1.00     13.2±1.13ms    30.5 MB/sec
semantic/typescript.js     1.00    235.3±6.17ms    40.9 MB/sec    1.04    244.6±6.96ms    39.3 MB/sec

Windows

group                      main                                   pr
-----                      ----                                   --
parser/babylon.max.js      1.00    124.7±1.59ms    82.8 MB/sec    1.03    129.0±5.77ms    80.0 MB/sec
parser/d3.js               1.00     15.4±0.35ms    35.5 MB/sec    1.08     16.7±1.74ms    32.7 MB/sec
parser/lodash.js           1.00      4.3±0.18ms   120.6 MB/sec    1.02      4.4±0.25ms   117.9 MB/sec
parser/pdf.js              1.00      8.6±0.14ms    46.9 MB/sec    1.04      8.9±0.31ms    45.1 MB/sec
parser/typescript.js       1.00    125.1±3.51ms    76.9 MB/sec    1.23   154.2±27.97ms    62.4 MB/sec
semantic/babylon.max.js    1.00    263.6±8.22ms    39.2 MB/sec    1.02    269.7±6.62ms    38.3 MB/sec
semantic/d3.js             1.00     31.0±1.29ms    17.6 MB/sec    1.12     34.6±1.52ms    15.8 MB/sec
semantic/lodash.js         1.00      6.4±0.10ms    80.5 MB/sec    1.03      6.6±0.17ms    78.0 MB/sec
semantic/pdf.js            1.00     13.4±0.32ms    30.1 MB/sec    1.17     15.7±0.79ms    25.6 MB/sec
semantic/typescript.js     1.00    231.8±5.96ms    41.5 MB/sec    1.07   246.8±10.29ms    39.0 MB/sec

@Boshen
Copy link
Member

Boshen commented Mar 24, 2023

I think this is in the right direction, shall we remove the OnceCell for now and focus on the parser itself on this PR?

I also realized by going through the jsdoc doc, there are three components:

  • jsdoc attachment to jsdoc targeting AST node
  • jsdoc parsing
  • jsdoc building for the entire module

So the overall architecture may be the following:

  1. The SemanticNode will have a flag indicating whether it has jsdoc comment - we want to minimize the bytes used by SemanticNode as it is constructed millions of times
  2. The JSDoc storage stores a map of NodeId or Span to the jsdoc comment span for this module
  3. The jsdoc is only parsed and cached when we call its API inside the linter:
  let parsed_jsdoc: Option<JSDoc> = ctx.semantic().jsdoc().get(node);

The get call does something like if !node.get().has_jsdoc() { return None }

I think I got an idea on how to do 1.

You can keep on working on 2 and 3 as current development setup is already in the right direction.

What do you think?

@Boshen
Copy link
Member

Boshen commented Mar 24, 2023

I started working on comment attachment here: 3b7469a

It seems I'll have to adjust some APIs on main, mainly to bring in trivias and source text into the semantic builder on initialization.

@Boshen
Copy link
Member

Boshen commented Mar 25, 2023

Updated with a commit:

We can now retrieve jsdoc on the following code:

/**
* @deprecated
**/
    function foo() {
    }

I'll keep on working with the builder by adding tests.

@shannonrothe You can focus on the parser without worrying about comment attachments now.

@Boshen
Copy link
Member

Boshen commented Apr 11, 2023

@shannonrothe Are you free for a round of review? Everything except the parser is ready to be merged. We can work on the parser in another around.

Copy link
Contributor Author

@shannonrothe shannonrothe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM @Boshen, just some minor questions 🚀

crates/oxc_semantic/src/jsdoc/mod.rs Show resolved Hide resolved
crates/oxc_semantic/src/jsdoc/builder.rs Outdated Show resolved Hide resolved
@Boshen Boshen merged commit 7899cf5 into main Apr 12, 2023
@Boshen Boshen deleted the jsdoc branch April 12, 2023 08:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants