CPLAT-11143 Emit a build error for mismatched components in the new boilerplate by greglittlefield-wf · Pull Request #496 · Workiva/over_react

greglittlefield-wf · 2020-05-15T18:56:14Z

Motivation

If a component isn't named correctly in the new boilerplate, it won't get associated with the factory and props class the author intended it to be.

Currently, this mismatched component is ignored and the component gets interpreted as a props mapview, and when the consumer invokes the builder, they get a runtime error.

It would be better if it failed the build.

Changes

When possible, increase v4_mixinBased confidence of components to above neutral, resulting in mismatched components emitting errors instead of getting silently ignored.
- Add tests
Update runtime error to mention the mismatched component case, for better debugging of edge case mismatched components (with neutral confidences) that don't trigger a build error
(Boyscouting) update punctuation in error messages

Release Notes

Emit a build error for mismatched components in the new boilerplate

Review

See CONTRIBUTING.md for more details on review types (+1 / QA +1 / +10) and code review process.

Please review:

QA Checklist

Tests were updated and provide good coverage of the changeset and other affected code
Manual testing was performed if needed
- Steps from PR author:
  - Verify in consumer repos that this does not cause any new failures
- Anything falling under manual testing criteria outlined in CONTRIBUTING.md

Merge Checklist

While we perform many automated checks before auto-merging, some manual checks are needed:

A Client Platform member has reviewed these changes
There are no unaddressed comments - this check can be automated if reviewers use the "Request Changes" feature
For release PRs - Version metadata in Rosie comment is correct

…uped with props/factory

aviary-wf · 2020-05-15T18:57:36Z

Security Insights

No security relevant content was detected by automated scans.

Action Items

Review PR for security impact; comment "security review required" if needed or unsure
Verify aviary.yaml coverage of security relevant code

Questions or Comments? Reach out on Slack: #support-infosec.

greglittlefield-wf · 2020-05-15T19:07:53Z

FYI @evanweible-wf @aaronstgeorge-wf

evanweible-wf · 2020-05-15T19:19:48Z

lib/src/builder/parsing/members_from_ast.dart

+        final confidences = VersionConfidences(
+          // If the component extends from a base class known to be supported by the new boilerplate,
+          // has no annotation, is not abstract, and does not have $isClassGenerated, then it's
+          // most likely intended to be part of a new boilerplate class component declaration.
+          //
+          // Make this `likely` so that components that don't get associated with factory/props
+          // due to naming issues aren't silently ignored.
+          v4_mixinBased: !classish.hasAbstractKeyword &&
+                  !_overridesIsClassGenerated(classish) &&
+                  mixinBoilerplateBaseClasses.contains(classish.superclass?.nameWithoutPrefix)
+              ? Confidence.likely
+              : Confidence.neutral,
+          v2_legacyBackwardsCompat: Confidence.neutral,
+          v3_legacyDart2Only: Confidence.neutral,


This is really cool

Thanks! I'm super happy that the confidence approach made this kind of change easy

willdrach-wk

One quick little thing but overall looks good!

lib/src/builder/parsing/declarations_from_members.dart

lib/src/builder/parsing/members_from_ast.dart

aaronlademann-wf

+1

greglittlefield-wf · 2020-05-20T16:24:32Z

Regression tests were added for the issue
The component that experienced this issue resulted in a build error
Building wdesk 1.5.3499 w/ this version of over_react resulted in no changes in generated Dart files
All classes outside of lib/ (e.g., not verified by the wdesk check) that could be affected by this change are part of boilerplate
https://sourcegraph.wk-dev.wdesk.org/search?q=lang:dart+%28%5Cn%7C%5E///.*%29%5Cn%5Eclass%5B%5E%5C%7B%3B%5D%2B%28%28%3F:Flux%29%3FUi%28%3F:Stateful%29%3FComponent%292+-repo:over_react+count:1000+file:%28app%7Cexample%7Ctest%7Ctool%29/&patternType=regexp

QA +1

willdrach-wk

just updating to remove the "request changes" - changes have been reflected

willdrach-wk

fine

greglittlefield-wf · 2020-05-20T17:04:24Z

@Workiva/release-management-p

rmconsole-wf

+1 from RM

greglittlefield-wf added 3 commits May 15, 2020 10:23

Emit build errors for most new boilerplate components that aren't gro…

b1dc74b

…uped with props/factory

Tidy up error message punctuation

a016add

Add additional info in assert message

b187a29

greglittlefield-wf requested review from aaronlademann-wf, corwinsheahan-wf, joebingham-wk and willdrach-wk May 15, 2020 18:56

rmconsole-wf added the Merge Requirements Unmet label May 15, 2020

Improve code comment

2a1adf4

evanweible-wf reviewed May 15, 2020

View reviewed changes

evanweible-wf previously approved these changes May 15, 2020

View reviewed changes

rmconsole-wf added Merge Requirements Met and removed Merge Requirements Unmet labels May 15, 2020

willdrach-wk suggested changes May 15, 2020

View reviewed changes

lib/src/builder/parsing/declarations_from_members.dart Show resolved Hide resolved

lib/src/builder/parsing/members_from_ast.dart Outdated Show resolved Hide resolved

aaronlademann-wf previously approved these changes May 15, 2020

View reviewed changes

Fix regex

85c7073

greglittlefield-wf dismissed stale reviews from aaronlademann-wf and evanweible-wf via 85c7073 May 15, 2020 20:59

rmconsole-wf added Merge Requirements Unmet and removed Merge Requirements Met labels May 15, 2020

aaronlademann-wf approved these changes May 15, 2020

View reviewed changes

rmconsole-wf added Merge Requirements Met and removed Merge Requirements Unmet labels May 15, 2020

joebingham-wk approved these changes May 15, 2020

View reviewed changes

rmconsole3-wf changed the title ~~Emit a build error for mismatched components in the new boilerplate~~ CPLAT-11143 Emit a build error for mismatched components in the new boilerplate May 15, 2020

willdrach-wk reviewed May 20, 2020

View reviewed changes

willdrach-wk approved these changes May 20, 2020

View reviewed changes

rmconsole-wf added the RM Ready label May 20, 2020

rmconsole-wf approved these changes May 20, 2020

View reviewed changes

rmconsole2-wf merged commit af3c8f5 into master May 20, 2020

rmconsole2-wf deleted the catch-mismatched-components branch May 20, 2020 17:04

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

CPLAT-11143 Emit a build error for mismatched components in the new boilerplate#496

CPLAT-11143 Emit a build error for mismatched components in the new boilerplate#496
rmconsole2-wf merged 5 commits intomasterfrom
catch-mismatched-components

greglittlefield-wf commented May 15, 2020

Uh oh!

aviary-wf commented May 15, 2020

Uh oh!

greglittlefield-wf commented May 15, 2020

Uh oh!

evanweible-wf May 15, 2020

Uh oh!

greglittlefield-wf May 15, 2020

Uh oh!

willdrach-wk left a comment

Uh oh!

Uh oh!

Uh oh!

aaronlademann-wf left a comment

Uh oh!

greglittlefield-wf commented May 20, 2020

Uh oh!

willdrach-wk left a comment •

edited

Loading

Uh oh!

willdrach-wk left a comment

Uh oh!

greglittlefield-wf commented May 20, 2020

Uh oh!

rmconsole-wf left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

Conversation

greglittlefield-wf commented May 15, 2020

Motivation

Changes

Release Notes

Review

QA Checklist

Merge Checklist

Uh oh!

aviary-wf commented May 15, 2020

Security Insights

Action Items

Uh oh!

greglittlefield-wf commented May 15, 2020

Uh oh!

evanweible-wf May 15, 2020

Choose a reason for hiding this comment

Uh oh!

greglittlefield-wf May 15, 2020

Choose a reason for hiding this comment

Uh oh!

willdrach-wk left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

aaronlademann-wf left a comment

Choose a reason for hiding this comment

Uh oh!

greglittlefield-wf commented May 20, 2020

Uh oh!

willdrach-wk left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

willdrach-wk left a comment

Choose a reason for hiding this comment

Uh oh!

greglittlefield-wf commented May 20, 2020

Uh oh!

rmconsole-wf left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

willdrach-wk left a comment •

edited

Loading