Skip to content

Commit

Permalink
Merge pull request #290 from Workiva/prop-requiredness-codemod-with-c…
Browse files Browse the repository at this point in the history
…ollection

FED-1885: Prop requiredness codemod
  • Loading branch information
rmconsole2-wf authored Aug 10, 2024
2 parents 7c6c6db + 6393501 commit d76eaa2
Show file tree
Hide file tree
Showing 57 changed files with 4,462 additions and 37 deletions.
8 changes: 7 additions & 1 deletion .github/workflows/dart_ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -34,13 +34,19 @@ jobs:
if: always() && steps.install.outcome == 'success'

- name: Validate formatting
run: dart format --set-exit-if-changed .
run: dart run dart_dev format --check
if: always() && steps.install.outcome == 'success' && matrix.sdk == '2.18.7'

- name: Analyze project source
run: dart analyze
if: always() && steps.install.outcome == 'success'

- name: Ensure checked-in generated files are up to date
run: |
dart run build_runner build --delete-conflicting-outputs
git diff --exit-code
if: always() && steps.install.outcome == 'success' && matrix.sdk == '2.19.6'

test:
runs-on: ubuntu-latest
strategy:
Expand Down
2 changes: 1 addition & 1 deletion analysis_options.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -5,4 +5,4 @@ analyzer:
missing_required_param: error
must_call_super: error
exclude:
- test/test_fixtures
- test/test_fixtures/**
15 changes: 15 additions & 0 deletions bin/null_safety_required_props.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
// Copyright 2024 Workiva Inc.
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

export 'package:over_react_codemod/src/executables/null_safety_required_props.dart';
11 changes: 11 additions & 0 deletions build.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
targets:
$default:
builders:
json_serializable:
generate_for:
include:
- '**.sg.dart'
source_gen|combining_builder:
options:
ignore_for_file:
- implicit_dynamic_parameter
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
import 'package:over_react_codemod/src/util.dart';
import 'package:over_react_codemod/src/util/component_usage.dart';
import 'package:analyzer/dart/ast/ast.dart';
import 'package:over_react_codemod/src/util/get_all_props.dart';
import 'package:over_react_codemod/src/vendor/over_react_analyzer_plugin/get_all_props.dart';
import 'package:pub_semver/pub_semver.dart';

import 'utils/class_component_required_fields.dart';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@

import 'package:over_react_codemod/src/util/component_usage.dart';
import 'package:analyzer/dart/ast/ast.dart';
import 'package:over_react_codemod/src/util/get_all_props.dart';
import 'package:over_react_codemod/src/util/get_all_state.dart';
import 'package:pub_semver/pub_semver.dart';

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ abstract class ClassComponentRequiredFieldsMigrator<
final Set<DefaultedOrInitializedDeclaration> fieldData = {};

void patchFieldDeclarations(
List<FieldElement> Function(InterfaceElement) getAll,
Iterable<FieldElement> Function(InterfaceElement) getAll,
Iterable<Assignment> cascadedDefaultPropsOrInitialState,
CascadeExpression node) {
for (final field in cascadedDefaultPropsOrInitialState) {
Expand All @@ -77,7 +77,7 @@ abstract class ClassComponentRequiredFieldsMigrator<
}

VariableDeclaration? _getFieldDeclaration(
List<FieldElement> Function(InterfaceElement) getAll,
Iterable<FieldElement> Function(InterfaceElement) getAll,
{required InterfaceElement propsOrStateElement,
required String fieldName}) {
// For component1 boilerplate its possible that `fieldEl` won't be found using `lookUpVariable` below
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,10 +33,12 @@ bool nonNullableHintAlreadyExists(TypeAnnotation type) {

const nonNullableHint = '/*!*/';

const lateHint = '/*late*/';

/// Whether the late hint already exists before [type]
bool requiredHintAlreadyExists(TypeAnnotation type) {
// Since the `/*late*/` comment is possibly adjacent to the prop declaration's doc comments,
// we have to recursively traverse the `precedingComments` in order to determine if the `/*late*/`
// comment actually exists.
return allComments(type.beginToken).any((t) => t.value() == '/*late*/');
return allCommentsForNode(type).any((t) => t.value() == lateHint);
}
112 changes: 112 additions & 0 deletions lib/src/dart3_suggestors/required_props/bin/aggregate.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,112 @@
// Copyright 2024 Workiva Inc.
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

import 'dart:async';
import 'dart:io';

import 'package:args/args.dart';
import 'package:collection/collection.dart';
import 'package:io/io.dart';
import 'package:logging/logging.dart';
import '../collect/aggregate.dart';
import '../collect/logging.dart';

/// Aggregates individual data files, like what the collect command does,
/// but as a standalone command.
///
/// This is leftover from before the collect command also aggregated data,
/// and is not publicly exposed, but is left in place just in case for
/// debugging purposes and potential future use.
///
/// Also outputs some additional statistics.
Future<void> main(List<String> args) async {
final argParser = ArgParser()
..addFlag('help', help: 'Print this usage information', negatable: false)
..addOption(
'output',
abbr: 'o',
help: 'The file to write output to.',
valueHelp: 'path',
defaultsTo: defaultAggregatedOutputFile,
);
final parsedArgs = argParser.parse(args);
if (parsedArgs['help'] as bool) {
print(argParser.usage);
exit(ExitCode.success.code);
}
final outputFile = parsedArgs['output']! as String;
final filesToAggregate = parsedArgs.rest;
if (filesToAggregate.isEmpty) {
print('Must specify files to aggregate.\n${argParser.usage}');
exit(ExitCode.usage.code);
}

initLogging();
final logger = Logger('prop_requiredness_aggregate');

logger.info('Loading results from files specified in arguments...');
final allResults = loadResultFiles(filesToAggregate);

{
// Gather some stats on how often different builder types show up.
final allUsages = allResults.expand((r) => r.usages);

final countsByBuilderType =
allUsages.countBy((u) => u.usageBuilderType.name);
File('counts_by_builder_type.json')
.writeAsStringSync(jsonEncodeIndented(countsByBuilderType));

final countsByBuilderTypeByMixin = allUsages
.multiGroupListsBy((u) => u.mixinData.map((e) => e.mixinId))
.map((mixinId, usages) =>
MapEntry(mixinId, usages.countBy((u) => u.usageBuilderType.name)));
File('counts_by_builder_type_by_mixin.json')
.writeAsStringSync(jsonEncodeIndented(countsByBuilderTypeByMixin));
}

logger.info('Aggregating data...');
final aggregated = aggregateData(allResults);
logger.info('Done.');

// logger.fine('Props mixins with the same name:');
// final mixinIdsByName = aggregated.mixinMetadata.mixinNamesById.keysByValues();
// mixinIdsByName.forEach((name, mixinIds) {
// if (mixinIds.length > 1) logger.fine('$name: ${mixinIds.map((id) => '\n - $id').join('')}');
// });

File(outputFile).writeAsStringSync(jsonEncodeIndented(aggregated));
logger.info('Wrote JSON results to $outputFile');
}

extension<E> on Iterable<E> {
Map<T, int> countBy<T>(T Function(E) getBucket) {
final counts = <T, int>{};
for (final element in this) {
final bucket = getBucket(element);
counts[bucket] = (counts[bucket] ?? 0) + 1;
}
return counts;
}

/// Like [groupListsBy] but allows elements to be added to multiple groups.
Map<T, List<E>> multiGroupListsBy<T>(Iterable<T> Function(E) keysOf) {
final groups = <T, List<E>>{};
for (final element in this) {
for (final key in keysOf(element)) {
groups.putIfAbsent(key, () => []).add(element);
}
}
return groups;
}
}
169 changes: 169 additions & 0 deletions lib/src/dart3_suggestors/required_props/bin/codemod.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,169 @@
// Copyright 2024 Workiva Inc.
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

import 'dart:convert';
import 'dart:io';

import 'package:args/args.dart';
import 'package:args/command_runner.dart';
import 'package:codemod/codemod.dart';
import 'package:over_react_codemod/src/dart3_suggestors/required_props/codemod/required_props_suggestor.dart';
import 'package:over_react_codemod/src/util.dart';
import 'package:over_react_codemod/src/util/args.dart';
import 'package:over_react_codemod/src/util/command_runner.dart';
import 'package:over_react_codemod/src/util/package_util.dart';

import '../codemod/recommender.dart';
import '../collect/aggregated_data.sg.dart';

abstract class _Options {
static const propRequirednessData = 'prop-requiredness-data';
static const privateRequirednessThreshold = 'private-requiredness-threshold';
static const privateMaxAllowedSkipRate = 'private-max-allowed-skip-rate';
static const publicRequirednessThreshold = 'public-requiredness-threshold';
static const publicMaxAllowedSkipRate = 'public-max-allowed-skip-rate';

static const all = {
propRequirednessData,
privateRequirednessThreshold,
privateMaxAllowedSkipRate,
publicRequirednessThreshold,
publicMaxAllowedSkipRate
};
}

abstract class _Flags {
static const trustRequiredAnnotations = 'trust-required-annotations';
static const all = {
trustRequiredAnnotations,
};
}

class CodemodCommand extends Command {
@override
String get description =>
"Adds null safety migrator hints to OverReact props using prop requiredness data from 'collect' command.";

@override
String get name => 'codemod';

@override
String get invocation => '$invocationPrefix [<options>]';

@override
String get usageFooter => '''
\nInstructions
============
1. First, run the 'collect' command to collect data on usages of props declared
in your package (see that command's --help for instructions).
$parentInvocationPrefix collect --help
2. Run this command within the package you want to update:
$invocationPrefix
3. Inspect the TODO comments left over from the codemod. If you want to adjust
any thresholds or re-collect data, discard changes before re-running the codemod.
4. Commit the changes made by the codemod.
5. Proceed with using the Dart null safety migrator tool to migrate your code.
6. Review TODO comments, adjusting requiredness if desired. You can use a
find-replace with the following regex to remove them:
${r'^ *// TODO\(orcm.required_props\):.+(?:\n *// .+)*'}
''';

CodemodCommand() {
argParser
..addOption(_Options.propRequirednessData,
help:
"The file containing prop requiredness data, collected via the 'over_react_codemod:collect' command.",
defaultsTo: 'prop_requiredness.json')
..addFlag(_Flags.trustRequiredAnnotations,
defaultsTo: true,
help:
'Whether to migrate @requiredProp and `@nullableRequiredProp` props to late required, regardless of usage data.'
'\nNote that @requiredProp has no effect on function components, so these annotations may be incorrect.')
..addOption(_Options.privateRequirednessThreshold,
defaultsTo: (0.95).toString(),
help:
'The minimum rate (0.0-1.0) a private prop must be set to be considered required.')
..addOption(_Options.privateMaxAllowedSkipRate,
defaultsTo: (0.2).toString(),
help:
'The maximum allowed rate (0.0-1.0) of dynamic usages of private mixins, for which data collection was skipped.'
'\nIf above this, all props in a mixin will be made optional (with a TODO comment).')
..addOption(_Options.publicRequirednessThreshold,
defaultsTo: (1).toString(),
help:
'The minimum rate (0.0-1.0) a public prop must be set to be considered required.')
..addOption(_Options.publicMaxAllowedSkipRate,
defaultsTo: (0.05).toString(),
help:
'The maximum allowed rate (0.0-1.0) of dynamic usages of public mixins, for which data collection was skipped.'
'\nIf above this, all props in a mixin will be made optional (with a TODO comment).');

argParser.addSeparator('Codemod options');
addCodemodArgs(argParser);
}

@override
Future<void> run() async {
final parsedArgs = this.argResults!;
final propRequirednessDataFile =
parsedArgs[_Options.propRequirednessData]! as String;
final codemodArgs = removeFlagArgs(
removeOptionArgs(parsedArgs.arguments, _Options.all), _Flags.all);

final packageRoot = findPackageRootFor('.');
await runPubGetIfNeeded(packageRoot);
final dartPaths = allDartPathsExceptHiddenAndGenerated();

final results = PropRequirednessResults.fromJson(
jsonDecode(File(propRequirednessDataFile).readAsStringSync()));
final recommender = PropRequirednessRecommender(
results,
privateRequirednessThreshold:
parsedArgs.argValueAsNumber(_Options.privateRequirednessThreshold),
privateMaxAllowedSkipRate:
parsedArgs.argValueAsNumber(_Options.privateMaxAllowedSkipRate),
publicRequirednessThreshold:
parsedArgs.argValueAsNumber(_Options.publicRequirednessThreshold),
publicMaxAllowedSkipRate:
parsedArgs.argValueAsNumber(_Options.publicMaxAllowedSkipRate),
);

exitCode = await runInteractiveCodemodSequence(
dartPaths,
[
RequiredPropsSuggestor(
recommender,
trustRequiredAnnotations:
parsedArgs[_Flags.trustRequiredAnnotations] as bool,
),
],
defaultYes: true,
args: codemodArgs,
additionalHelpOutput: argParser.usage,
);
}
}

extension on ArgResults {
num argValueAsNumber(String name) => num.parse(this[name]);
}
Loading

0 comments on commit d76eaa2

Please sign in to comment.