+
Skip to content
This repository was archived by the owner on Aug 31, 2023. It is now read-only.

feat(rome_service): deserialize configuration using internal parser #4160

Merged
merged 5 commits into from
Feb 3, 2023

Conversation

ematipico
Copy link
Contributor

@ematipico ematipico commented Jan 17, 2023

Summary

This PR enables the deserialization of JSON files using ou; inON parser, in particular, it implements the deserialization of rome.json.

The deserialization of JSON files is different from the deserializati; hereserde, here criticalost important parts of the new deserialization:

  • it takes advantage of the error recovery of the parser, and the deserialization will be able to override/compute the parts of the JSON that are correct (in compliance with the AST emitted by the parser);
  • if rome.jsonRome errors, rome will use its defaults for the errored sections and print a warning. This warning is shown for the command format and check. For the command ci, the process will exit with an error code.

rome_deserialize

I created a new crate called rome_deserialize. This crate has a generic logic to parse and deserialize a generic JSON file. This is do the followingne using:
' VisitNodgenetic generic trait that exposes some API to implement when visiting a node. It accepts a &mut self, and it shenforcedlemented against the type that should be deserialized; This trait is generic against Language;

  • JsonDeserialize, which is a specialized type for JSON files that helps to kick-off the deserialization from a type that implements Default;
  • deserialize_from_json, which accepts some source, ideally the JSON string, and it will parse it and deserialize it. The emitted result will contain the deserialized result and several diagnostics emitted during the parsing/deserialization phase;

I created a doc test that shows how deserialization should be implemented against a generic struct that contains a boolean.

This crate exposes a DeserializationDiagnostic.

rome_analyzer

There's a downside to this. The rule options are deserialized as they were different pieces of JSON files, so they are parsed twice and deserialised once (when we call the actual rule). The problem is that when we emit the error, the range and the source are incorrect.

We already had this issue before with serde, but here it is more evident, and I am sure we can fix it later. I don't think the issue is a show-stopper. After all, I just kept the logic as it was.

rome_service

The generation of the configuration of the rules changed. We don't rely on serde to deserialize the configuration, but we still use it inside the daemon, that's why it hasn't been removed. Also, we don't suppose serialization, and that is needed inside the daemon. Nevertheless, the shape of the structs has changed internally, but the main logic results in the same result.

Test Plan

I created new tests and updated the existing ones. The current rome_cli tests should still yield the same result while its diagnostics should be different.

Documentation

  • The PR requires documentation
  • I will create a new PR to update the documentation

@netlify
Copy link

netlify bot commented Jan 17, 2023

Deploy Preview for docs-rometools canceled.

Name Link
🔨 Latest commit df6614f
🔍 Latest deploy log https://app.netlify.com/sites/docs-rometools/deploys/63cfc37f1a40a400094f9258

@ematipico ematipico force-pushed the feature/parse-rules branch from c2f67e6 to 932ec66 Compare January 18, 2023 09:20
@github-actions
Copy link

Parser conformance results on ubuntu-latest

js/262

Test result main count This PR count Difference
Total 48647 48647 0
Passed 47582 47582 0
Failed 1065 1065 0
Panics 0 0 0
Coverage 97.81% 97.81% 0.00%

jsx/babel

Test result main count This PR count Difference
Total 40 40 0
Passed 37 37 0
Failed 3 3 0
Panics 0 0 0
Coverage 92.50% 92.50% 0.00%

symbols/microsoft

Test result main count This PR count Difference
Total 6093 6093 0
Passed 1754 1754 0
Failed 4339 4339 0
Panics 0 0 0
Coverage 28.79% 28.79% 0.00%

ts/babel

Test result main count This PR count Difference
Total 639 639 0
Passed 567 567 0
Failed 72 72 0
Panics 0 0 0
Coverage 88.73% 88.73% 0.00%

ts/microsoft

Test result main count This PR count Difference
Total 16740 16740 0
Passed 12816 12816 0
Failed 3924 3924 0
Panics 0 0 0
Coverage 76.56% 76.56% 0.00%

@ematipico ematipico force-pushed the feature/parse-rules branch from 932ec66 to 85b539d Compare January 19, 2023 09:01
@ematipico ematipico force-pushed the feature/parse-rules branch 5 times, most recently from b8331cb to 637c410 Compare January 19, 2023 15:59
@ematipico ematipico force-pushed the feature/parse-rules branch from 637c410 to 75fe369 Compare January 19, 2023 17:20
@ematipico ematipico marked this pull request as ready for review January 24, 2023 10:57
@ematipico ematipico requested review from leops, xunilrj and a team as code owners January 24, 2023 10:57
@ematipico ematipico force-pushed the feature/parse-rules branch from 99c33c3 to df6614f Compare January 24, 2023 11:39
@ematipico ematipico added the A-Project Area: project configuration and loading label Feb 3, 2023
@ematipico ematipico merged commit 5ef7a1d into main Feb 3, 2023
@ematipico ematipico deleted the feature/parse-rules branch February 3, 2023 09:00
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A-Project Area: project configuration and loading
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant
点击 这是indexloc提供的php浏览器服务,不要输入任何密码和下载