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

Add lib/fix_data.yaml for automatic migration with dart fix. #198

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

jonasfj
Copy link

@jonasfj jonasfj commented Oct 15, 2024

These fixes will also show up as "quick fixes" in IDEs. Users do not need to run dart fix --apply manually.

Example:

import 'package:json_schema/json_schema.dart';

void main() {
  final schema = JsonSchema.empty();
  schema.validateWithResults(null);
  final validator = Validator(schema);
  validator.validateWithResults(null);
}

Will be updated by dart fix --apply to:

import 'package:json_schema/json_schema.dart';

void main() {
  final schema = JsonSchema.empty();
  schema.validate(null);
  final validator = Validator(schema);
  validator.validate(null);
}

Ultimate problem:

@Deprecated message on validateWithResults was not automatically migrated.

How it was fixed:

Added lib/fix_data.yaml with data driven fixes.

Testing suggestions:

It's possible to make a test_fixes/ folder with golden files that can be tested with dart fix --compare-to-golden however this is probably overkill. See testing.

I manually tested this. There is probably no harm in leaving this in place even after 5.0 is released, as it'll probably still facilitate quick fixes for anyone upgrading.

Potential areas of regression:

If these data driven fixes stop working, they aren't likely to cause much harm (they simply won't cause fixes to be suggested).


I mostly made this because I think fix_data.yaml is too cool, not to be used 🤣

These fixes will also show up as "quick fixes" in IDEs.
Users do not need to run `dart fix --apply` manually.

**Example:**
```dart
import 'package:json_schema/json_schema.dart';

void main() {
  final schema = JsonSchema.empty();
  schema.validateWithResults(null);
  final validator = Validator(schema);
  validator.validateWithResults(null);
}
```

Will be updated by `dart fix --apply` to:
```dart
import 'package:json_schema/json_schema.dart';

void main() {
  final schema = JsonSchema.empty();
  schema.validate(null);
  final validator = Validator(schema);
  validator.validate(null);
}
```
@aviary3-wk
Copy link

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.

@michaelcarter-wf
Copy link

I'd love to see a test for this as well if you can do it, since this would be our first usage of this feature and want to make sure it doesn't regress as we add more. Thanks for your contribuitions!

@jonasfj
Copy link
Author

jonasfj commented Oct 21, 2024

I added some tests, but testing this might be overkill.

If these tests breaks at some point, it might be better to delete them, than trying to maintain the tests. I'm not sure if having untested lib/fix_data.yaml entries is actually that bad.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants