[PM-33358] Loosen parsing strictness for some field types

[rbartlensky]

👋 Me again! I have another set of patches that should help with parsing non-conforming CXF.

🎟️ Tracking

📔 Objective

In the wild, I noticed some providers don't respect the spec and
send string where concealed-string is expected, and so on. I
think it would be a good idea to accept some of these inconsistencies.

My commit messages have a bit more context around the types of issues
I've encountered.

⏰ Reminders before review

• Contributor guidelines followed
• All formatters and local linters executed and passed
• Written new unit and / or integration tests where applicable
• Protected functional changes with optionality (feature flags)
• Used internationalization (i18n) for all UI strings
• CI builds passed
• Communicated to DevOps any deployment requirements
• Updated any necessary documentation (Confluence, contributing docs) or informed the documentation team

🦮 Reviewer guidelines

• 👍 (:+1:) or similar for great changes
• 📝 (:memo:) or ℹ️ (:information_source:) for notes or general info
• ❓ (:question:) for questions
• 🤔 (:thinking:) or 💭 (:thought_balloon:) for more open inquiry that's not quite a confirmed issue and could potentially benefit from discussion
• 🎨 (:art:) for suggestions / improvements
• ❌ (:x:) or ⚠️ (:warning:) for more significant problems or concerns needing attention
• 🌱 (:seedling:) or ♻️ (:recycle:) for future improvements or indications of technical debt
• ⛏ (:pick:) for minor or nitpick changes

[bitwarden-bot]

Thank you for your contribution! We've added this to our internal tracking system for review.
ID: PM-33358
Link: https://bitwarden.atlassian.net/browse/PM-33358

Details on our contribution process can be found here: https://contributing.bitwarden.com/contributing/pull-requests/community-pr-process.

[Hinton]

Hey, this is #122.

I'm still pondering the optimal solution here, and while casting is one approach which decreases the likelihood of incorrect values it can't handle every scenario. And I think discarding non cast-able values would be unfortunate.

Another approach that I think I'm leaning towards is to accept any EditableField and leave it to downstream to handle this would guarantee parsing compatibility but result in more complex implementations and remove type safety on exports for using the most appropriate type. I suspect we could make it more ergonomic by having a Expected type which defines the expected type and offers an enum for remainders, Expected<EditableFieldDate, Others>, but I need to explore it properly.

[rbartlensky]

Hey, this is #122.

Somehow I missed this when I checked the issue tracker... sorry about that!

I'm still pondering the optimal solution here, and while casting is one approach which decreases the likelihood of incorrect values it can't handle every scenario. And I think discarding non cast-able values would be unfortunate.

Another approach that I think I'm leaning towards is to accept any EditableField and leave it to downstream to handle this would guarantee parsing compatibility but result in more complex implementations and remove type safety on exports for using the most appropriate type. I suspect we could make it more ergonomic by having a Expected type which defines the expected type and offers an enum for remainders, Expected<EditableFieldDate, Others>, but I need to explore it properly.

I would hate to lose type-safety on exports as well. The Expected<EditableFieldDate, Others> sounds like a better approach tbh. Happy to explore that in another patch if you'd like.

[rbartlensky]

@Hinton I took a stab at your suggestion and here is what I have so far. There are still some TODOs, but I didn't want to go too deep without consulting with you. Let me know what you think!

[Hinton]

@rbartlensky I think the pattern should work, not sure if we need the Expected(ExpctedInner) wrappers or if we can expose the enum directly? If you have some more time this week do please open a PR and I'll take a look beginning next week. If not I should have some time next week to explore it myself.

[rbartlensky]

@rbartlensky I think the pattern should work, not sure if we need the Expected(ExpctedInner) wrappers or if we can expose the enum directly? If you have some more time this week do please open a PR and I'll take a look beginning next week. If not I should have some time next week to explore it myself.

Opened #127 instead. The struct is used to ensure it's impossible to create a non-conforming CXF by accident. Closing this one in favor of #127