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

MatchingError output is confusing #50

Open
XAMPPRocky opened this issue Oct 7, 2024 · 4 comments
Open

MatchingError output is confusing #50

XAMPPRocky opened this issue Oct 7, 2024 · 4 comments

Comments

@XAMPPRocky
Copy link
Contributor

The error output I get from this error doesn't really give me actionable information. I think it would be good to put some work in including more information to make errors more readable and accessible.

Two libraries that would help a lot here

Another alternative are the Chumsky/Ariadne combo which would require moving away from nom, but adding good errors might require extensive plumbing regardless.


Rasn compiler error:
Encountered error while parsing MatchingError(Tag) - Error matching ASN syntax at while parsing: CA-ParametersNR-v1700 ::= SEQUENCE {
  -- R1 23-9-1: Basic Features of Further Enhanced Port-Selection Type II Codebook (FeType-II) per band combination information
    codebookParametersfetype2PerBC-r17               CodebookParametersfetype2PerBC-r17           OPTIONAL,
  -- R4 18-4: Support of e

@6d7a
Copy link
Member

6d7a commented Oct 8, 2024

Thank you for your suggestions. What information would you like to see in a meaningful compiler error? Pinpointing the exact location might not even need a lot of changes.

@XAMPPRocky
Copy link
Contributor Author

Apart from standard info like line number and spans. I think when designing the errors, the perspective should be "What does the user need to do to fix this?" for example right now this error doesn't tell what is failing to match, do I need fix a type name? It should point at exactly where the problem is and how it can be fixed, e.g. if it's a typo it should tell me whether there's something that closely matches the type name present.

@XAMPPRocky
Copy link
Contributor Author

Another thing that would help with this is adding snapshot testing that adds tests with known bad modules that test what different possible errors look like, that will make it a lot easier to prevent errors from regressing, and make it easy to see when a change improves an error message.

@6d7a
Copy link
Member

6d7a commented Nov 12, 2024

I like the idea of snapshot testing and I would like to get the rasn-compiler-test there. As for matching errors, I made some improvements that should give users a better idea of where the problematic ASN1 is located in a module. I do not use a third-party crate for formatting at this point, because I wanted to defer adding another dependency until the next iteration of error improvements. I hope the changes provide some value. Also feel free to improve the error handling further.

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

No branches or pull requests

2 participants