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

Draft: pretty-printer tests with random generated AST Expressions #1248

Draft
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

xpyctumo
Copy link

@xpyctumo xpyctumo commented Dec 24, 2024

Issue

Towards #642

Checklist

  • I have updated CHANGELOG.md
  • I have documented my contribution in docs/ and made the build locally
  • I have added tests to demonstrate the contribution is correctly implemented: this usually includes both positive and negative tests, showing the happy path(s) and featuring intentionally broken cases
  • I have run all the tests locally and no test failure was reported
  • I have run the linter, formatter and spellchecker
  • I did not do unrelated and/or undiscussed refactorings

Plan

  • AstExpressionPrimary
  • AstOpBinary
  • AstOpUnary
  • AstConditional
  • AstMethodCall
  • AstFieldAccess
  • AstStaticCall
  • AstStructInstance
  • AstNumber
  • AstBoolean
  • AstId
  • AstNull
  • AstInitOf
  • AstString

@xpyctumo xpyctumo self-assigned this Dec 24, 2024
@xpyctumo xpyctumo requested a review from a team as a code owner December 24, 2024 14:27
@anton-trunov anton-trunov marked this pull request as draft December 24, 2024 14:31
@anton-trunov
Copy link
Member

anton-trunov commented Dec 24, 2024

when I see something like this, I immediately think of adding shrinking (fast-check should support it)

(+0b100100110001001100110111100100000101101111000011101011010101101101011110010000001000101100100000011000111001001110101001111011111100001101111110011000101010010001100000100100011010000100110011001111001000111001011101000100011011100111001110 ? 0xf39c6286c40970dcb2a321a1c21e4549de1a2d60bcc7ff84622639b35024a7f || 0o3235042541620525012122214562100130425412434124566713521630100545344175751456215504064 - -0o7777777777777777777777777777777777777777777777777777777777777777777777777777777777743 : 8 - (16227299991510006730850008471880976967479204885141379564359356953639308462371 == 0o5735742350164242162734544666035326212173453606274734532135112142107426251434140545063)) ? 0b10111100010010010001011110101011101010001000111101110001100011100001101000001011001111111101110100100011001000001001001111011110100011001100011010100111110010110110010011110110010001101000100111000101011110110110110010100100010011010011 + 0b11001101101101011111001101010011101111110011111110101001101100111000110011110110011010111001101011001111001111110001100110000010001001101011100010111101000010100000010001011100101110010100100101100101101110110110111100111100110111000101011111011001100010 + 26 + (0x157e944ffa07c17e8b222faf8abfab932cd54c213784eca386a32755e232289a ? 0o2667602156054031504566507661155302000265110317032415711565650770337656725250250767327 << 0o5520154115374334170135502317743632312724505264461136361671447006177506075556352660004 : 0o6624600205764760715715350617562272506065501073701250447411343437104466044353352053407) : 0b1

even shrinking integer literals would already help a lot here

@xpyctumo
Copy link
Author

xpyctumo commented Dec 25, 2024

when I see something like this, I immediately think of adding shrinking (fast-check should support it)

(+0b100100110001001100110111100100000101101111000011101011010101101101011110010000001000101100100000011000111001001110101001111011111100001101111110011000101010010001100000100100011010000100110011001111001000111001011101000100011011100111001110 ? 0xf39c6286c40970dcb2a321a1c21e4549de1a2d60bcc7ff84622639b35024a7f || 0o3235042541620525012122214562100130425412434124566713521630100545344175751456215504064 - -0o7777777777777777777777777777777777777777777777777777777777777777777777777777777777743 : 8 - (16227299991510006730850008471880976967479204885141379564359356953639308462371 == 0o5735742350164242162734544666035326212173453606274734532135112142107426251434140545063)) ? 0b10111100010010010001011110101011101010001000111101110001100011100001101000001011001111111101110100100011001000001001001111011110100011001100011010100111110010110110010011110110010001101000100111000101011110110110110010100100010011010011 + 0b11001101101101011111001101010011101111110011111110101001101100111000110011110110011010111001101011001111001111110001100110000010001001101011100010111101000010100000010001011100101110010100100101100101101110110110111100111100110111000101011111011001100010 + 26 + (0x157e944ffa07c17e8b222faf8abfab932cd54c213784eca386a32755e232289a ? 0o2667602156054031504566507661155302000265110317032415711565650770337656725250250767327 << 0o5520154115374334170135502317743632312724505264461136361671447006177506075556352660004 : 0o6624600205764760715715350617562272506065501073701250447411343437104466044353352053407) : 0b1

even shrinking integer literals would already help a lot here

I'm thinking about that right now. It might become more user- friendly values

value: fc.bigInt({ min: -1_000_000_000n, max: 1_000_000_000n })

Comment on lines 59 to 60
left: expression,
right: expression,
Copy link
Member

Choose a reason for hiding this comment

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

These two need to be different expressions in general

Copy link
Author

Choose a reason for hiding this comment

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

Also fixed

Comment on lines 23 to 24
id: fc.constant(0),
loc: fc.constant(dummySrcInfo),
Copy link
Member

Choose a reason for hiding this comment

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

I'd add a wrapper to automatically add these id and loc to the generated AST nodes

Copy link
Author

Choose a reason for hiding this comment

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

All generators wrapped by dummyAstNode now

Comment on lines 68 to 70
condition: expression,
thenBranch: expression,
elseBranch: expression,
Copy link
Member

Choose a reason for hiding this comment

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

These need to be 3 different expressions

Copy link
Author

Choose a reason for hiding this comment

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

Fixed, every expression is new-generated

@xpyctumo xpyctumo force-pushed the fast-check branch 2 times, most recently from 80ca1e6 to 88786b8 Compare December 26, 2024 17:19
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.

3 participants