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

RDART-992: Const initializer evaluation is too simple #1607

Merged
merged 5 commits into from Mar 28, 2024

Conversation

nielsenko
Copy link
Contributor

@nielsenko nielsenko commented Mar 27, 2024

Fix #1606

@nielsenko nielsenko marked this pull request as ready for review March 27, 2024 10:57
@nielsenko nielsenko changed the title Const initializer evaluation is too simple RDART-992: Const initializer evaluation is too simple Mar 27, 2024
Copy link

coveralls-official bot commented Mar 27, 2024

Pull Request Test Coverage Report for Build 8471153652

Details

  • 7 of 8 (87.5%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-0.001%) to 86.227%

Changes Missing Coverage Covered Lines Changed/Added Lines %
packages/realm_generator/lib/src/field_element_ex.dart 7 8 87.5%
Totals Coverage Status
Change from base Build 8392307484: -0.001%
Covered Lines: 5835
Relevant Lines: 6767

💛 - Coveralls

@nielsenko nielsenko requested review from papafe and elle-j March 27, 2024 11:21
Copy link
Member

@nirinchev nirinchev left a comment

Choose a reason for hiding this comment

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

Looks good, a minor suggestion on my end.

Comment on lines 395 to 398
if (initExpression is Literal) return true;
if (initExpression is InstanceCreationExpression) return initExpression.isConst;
if (initExpression is PrefixExpression) return _isValidFieldInitializer(initExpression.operand);
return false;
Copy link
Member

Choose a reason for hiding this comment

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

Can we rewrite this with a switch expression? I'm not a huge fan of series of 1-line ifs.

Copy link
Contributor

@papafe papafe left a comment

Choose a reason for hiding this comment

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

looks good

Copy link
Member

@elle-j elle-j left a comment

Choose a reason for hiding this comment

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

Good fix 👍

CHANGELOG.md Outdated Show resolved Hide resolved
nielsenko and others added 3 commits March 28, 2024 17:44
Co-authored-by: LJ <81748770+elle-j@users.noreply.github.com>
@nielsenko nielsenko merged commit 4dab114 into main Mar 28, 2024
56 checks passed
@nielsenko nielsenko deleted the kn/fix-const-init-bug branch March 28, 2024 18:29
papafe added a commit that referenced this pull request Apr 8, 2024
* main:
  RDART-996: Don't serialize backlinks (#1617)
  Update README.md (#1608)
  RDART-983: Refactor how we open dynamic library to give better error message (#1614)
  RDART-991: Rename Key enum (#1613)
  RDART-992: Handle Identifer expression as well (#1612)
  Add workflow_dispatch for development
  Common setup script (#1610)
  RDART-992: Const initializer evaluation is too simple (#1607)

# Conflicts:
#	CHANGELOG.md
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 27, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Realm generator rejects const values saying initialization must be const
4 participants