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

feat(rome_service): parse configuration via JSON parser #4043

Merged
merged 9 commits into from
Jan 1, 2023

Conversation

ematipico
Copy link
Contributor

@ematipico ematipico commented Dec 12, 2022

Summary

Part of #4042

This PR implements the following:

  • implement a generic infrastructure to visit the configuration
  • implement a specialized infrastructure to visit the JSON configuration
  • visit, parse and validate the rome.json type
  • visit, parse and validate the rome.json#files type
  • visit, parse and validate the rome.json#formatter type
  • visit, parse and validate the rome.json#javascript type

A new trait called VisitConfigurationNode has been created, which is generic against a L: Language. This will allow us to implement the parsing of Configuration against multiple languages. So far, I have created only three functions:

  • visit_member_name
  • visit_member_value
  • visit_map

Initially, I had created more functions - like visit_string_node - but I ended up not using them, so I decided to keep only the ones I used. We can add new functions if the need arises.

A new trait called VisitConfigurationNodeAsJson adds functions and patterns I used throughout the implementors.

ConfigurationError has been renamed ConfigurationErrorKind, and a new struct called ConfigurationDiagnostic has been created, which is in charge of creating dedicated diagnostics based on the situation encountered. ConfigurationErrorKind is now part of ConfigurationDiagnostic.

This PR doesn't implement the parsing and validation of the various groups we have inside linter.rules.<group>. There's a lot of generated code there, and I thought I should stop there and make another PR just for this.

This PR doesn't handle bogus nodes and partially valid documents.

Test Plan

I created a new test suite that reads JSON files as input and prints the diagnostics encountered. The test suite, for now, is meant only for invalid cases. I have tried to cover the majority of "special" cases we have inside our configuration.

Open questions

For now, I left out on purpose all cases where we could find Bogus nodes. I suppose the question is what we should do with partially invalid configuration and how diagnostics should be collected.

@netlify
Copy link

netlify bot commented Dec 12, 2022

Deploy Preview for docs-rometools ready!

Name Link
🔨 Latest commit 041be09
🔍 Latest deploy log https://app.netlify.com/sites/docs-rometools/deploys/63b1db61e33fa80008e0b770
😎 Deploy Preview https://deploy-preview-4043--docs-rometools.netlify.app/playground
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@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 47580 47580 0
Failed 1067 1067 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 562 562 0
Failed 77 77 0
Panics 0 0 0
Coverage 87.95% 87.95% 0.00%

ts/microsoft

Test result main count This PR count Difference
Total 16740 16740 0
Passed 12814 12814 0
Failed 3926 3926 0
Panics 0 0 0
Coverage 76.55% 76.55% 0.00%

@ematipico ematipico marked this pull request as ready for review December 12, 2022 13:15
@ematipico ematipico requested review from leops and a team as code owners December 12, 2022 13:15
@ematipico ematipico added the A-Project Area: project configuration and loading label Dec 12, 2022
@rome rome deleted a comment from calibre-analytics bot Dec 22, 2022
@ematipico ematipico merged commit 59a2f8c into main Jan 1, 2023
@ematipico ematipico deleted the feat/config-json-parse branch January 1, 2023 22:30
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.

None yet

4 participants