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

Allow analyzer 1.0 #736

Merged
merged 21 commits into from
May 11, 2022
Merged
Show file tree
Hide file tree
Changes from 16 commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
0dc0683
Allow analyzer 1.0
robbecker-wf Feb 23, 2022
2283fd1
more ranges widened
robbecker-wf Feb 24, 2022
11809d2
use git refs
robbecker-wf Feb 24, 2022
7be8925
remove git overrides now that pkgs are published
robbecker-wf Feb 24, 2022
65939e4
use a range for analyzer, widen plugin deps
robbecker-wf Feb 25, 2022
ed98e76
restore plugin pubspec changes
robbecker-wf Feb 25, 2022
45612aa
restore analyzer 1.0 since build 2.0 is required
robbecker-wf Mar 4, 2022
49df880
Merge remote-tracking branch 'origin/master' into analyzer1
robrbecker May 1, 2022
da645a2
Revert "restore analyzer 1.0 since build 2.0 is required"
greglittlefield-wf May 9, 2022
28b05c4
Lower lower bound of analyzer as much as possible
greglittlefield-wf May 9, 2022
da63635
Add CI validation for each supported analyzer version
greglittlefield-wf May 9, 2022
ea26aa9
Support build 1.x and 2.x at the same time
greglittlefield-wf May 10, 2022
54169b2
Fix syntax, disable fail-fast
greglittlefield-wf May 10, 2022
1f865aa
Add Dart setup to matrix
greglittlefield-wf May 10, 2022
fff6dfe
Remove ^0.42.0 from matrix
greglittlefield-wf May 10, 2022
0e3d011
Revert analyzer_plugin pubspec.yaml changes, for now
greglittlefield-wf May 10, 2022
320d7fa
Update tool/validate_analyzer_version.sh
robbecker-wf May 10, 2022
4cdd736
Move script into separate CI steps for improved readability
greglittlefield-wf May 10, 2022
f582396
Raise built_redux upper bound
greglittlefield-wf May 10, 2022
6509584
Output the updated version constraint in CI
greglittlefield-wf May 10, 2022
7c7dead
Add arguments to parameters made required in built_redux 8.0.0
greglittlefield-wf May 10, 2022
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 29 additions & 0 deletions .github/workflows/dart_ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,35 @@ jobs:
run: dart pub run dart_dev test --build-args="-r" -P dart2js
if: always() && steps.install.outcome == 'success' && steps.build.outcome == 'success'

validate_analyzer:
runs-on: ubuntu-latest
strategy:
fail-fast: false
matrix:
sdk: [ 2.13.4, stable ]
analyzer:
- ^0.40.0
- ^0.41.0
# No ^0.42.0 since that line only contains 0.42.0-nullsafety.0, which is the 1.0.0 prerelease
- ^1.0.0
# For stable, only validate analyzer ^1.0.0. Otherwise, validate all analyzer versions.
exclude:
- sdk: stable
include:
- sdk: stable
analyzer: ^1.0.0
steps:
- uses: actions/checkout@v2
- uses: dart-lang/setup-dart@v0.2
with:
sdk: ${{ matrix.sdk }}

- name: Print Dart SDK version
run: dart --version

- name: Validate dependency resolution, static analysis, and builder when using analyzer ${{ matrix.analyzer }}
run: ./tool/validate_analyzer_version.sh "${{ matrix.analyzer }}"

analyzer_plugin:
runs-on: ubuntu-latest
defaults:
Expand Down
23 changes: 22 additions & 1 deletion lib/src/builder/builder.dart
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ class OverReactBuilder extends Builder {

// Generate over_react code for each part file of the input library.
for (final part in parts) {
final partId = AssetId.resolve(
final partId = resolveAssetId(
part.uri.stringValue,
from: buildStep.inputId);
if (!await buildStep.canRead(partId)) {
Expand Down Expand Up @@ -222,3 +222,24 @@ class OverReactBuilder extends Builder {
await buildStep.writeAsString(outputId, output);
}
}

/// A compatibility layer for [AssetId.resolve],
/// which in build <2.0.0 accepts a String for the first argument and
/// and in build >=2.0.0 accepts a Uri for the first argument.
///
/// This function allows us to support build 1.x and 2.x
///
// TODO remove once we're off of build 1.x
AssetId resolveAssetId(String uri, {AssetId from}) {
try {
// `as dynamic` is necessary to prevent compile errors.
// This ignore is to prevent analysis implicit cast errors.
// ignore: argument_type_not_assignable
return AssetId.resolve(uri as dynamic, from: from);
} catch (_) {
// `as dynamic` is necessary to prevent compile errors.
// This ignore is to prevent analysis implicit cast errors.
// ignore: argument_type_not_assignable
return AssetId.resolve(Uri.parse(uri) as dynamic, from: from);
}
}
23 changes: 10 additions & 13 deletions pubspec.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -7,14 +7,11 @@ environment:

dependencies:
collection: ^1.14.11
# Dart 2.7 needs 0.39.x to resolve,
# and Dart 2.12+ needs 0.42.x to resolve to build_web_compilers 2.12.0, etc.
# to enable proper opting out of null-safety.
analyzer: '>=0.39.0 <0.42.0'
analyzer: '>=0.40.0 <2.0.0'
build: '>=1.0.0 <3.0.0'
built_redux: ^7.4.2
Copy link
Contributor

Choose a reason for hiding this comment

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

built_redux < 8.0.0 does not allow analyzer 1.0. Should this be expanded?

Suggested change
built_redux: ^7.4.2
built_redux: ">=7.4.2 <9.0.0"

Copy link
Contributor

Choose a reason for hiding this comment

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

Based on this CI run it seems like it does:
Screen Shot 2022-05-10 at 10 52 37 AM

Though, I'm not opposed to raising the bound on built_redux so this package isn't a bottleneck. Let me take a look

Copy link
Member Author

Choose a reason for hiding this comment

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

yeah we've backpatched built_redux v7 to allow analyzer v1

built_value: ^8.0.0
dart_style: ^1.2.5
dart_style: '>=1.2.5 <3.0.0'
js: ^0.6.1+1
logging: '>=0.11.3+2 <2.0.0'
meta: ">=1.1.6 <1.7.0" # Workaround to avoid https://github.com/dart-lang/sdk/issues/46142
Expand All @@ -26,24 +23,24 @@ dependencies:
w_common: ^1.13.0
w_flux: ^2.10.4
platform_detect: ^1.3.4
quiver: ">=0.25.0 <3.0.0"
quiver: ">=0.25.0 <4.0.0"
redux_dev_tools: ">=0.4.0 <0.6.0"

dev_dependencies:
build_resolvers: ^1.0.5
build_resolvers: '>=1.0.5 <3.0.0'
build_runner: '>=1.7.1 <3.0.0'
build_test: ">=0.10.9 <2.0.0"
build_web_compilers: ^2.5.1
build_test: ">=0.10.9 <3.0.0"
build_web_compilers: '>=2.12.0 <4.0.0'
built_value_generator: ^8.0.0
dart_dev: ^3.6.4
dependency_validator: ^2.0.0
glob: ^1.2.0
io: ^0.3.2+1
dependency_validator: '>=2.0.0 <4.0.0'
glob: '>=1.2.0<3.0.0'
io: '>=0.3.2+1 <2.0.0'
mockito: ^4.1.1
over_react_test: ^2.10.2
pedantic: ^1.8.0
test: ^1.15.7
yaml: ^2.2.1
yaml: '>=2.2.1 <4.0.0'

workiva:
core_checks:
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

25 changes: 25 additions & 0 deletions tool/set_analyzer_constraint.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
import 'dart:io';

final analyzerConstraintPattern =
RegExp(r'^( +analyzer:\s*).+', multiLine: true);

void main(List<String> args) {
if (args.length != 1) {
throw ArgumentError(
'Expected a single arg for the new analyzer constraint. Args: $args');
}
final newAnalyzerConstraint = args.single;

final pubspec = File('pubspec.yaml');
final pubspecContents = pubspec.readAsStringSync();

final matches = analyzerConstraintPattern.allMatches(pubspecContents);
if (matches.length != 1) {
throw Exception(
'Expected 1 analyzer dependency match in ${pubspec.path}, but found ${matches.length}.');
}

pubspec.writeAsStringSync(pubspecContents.replaceFirstMapped(
analyzerConstraintPattern,
(match) => '${match.group(1)}$newAnalyzerConstraint'));
}
22 changes: 22 additions & 0 deletions tool/validate_analyzer_version.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
#!/usr/bin/env bash

# Fail this script if any command fails
set -e
# Print commands as they run
greglittlefield-wf marked this conversation as resolved.
Show resolved Hide resolved
set -x

analyzer_constraint="$1"

echo "Validating analyzer version constraint: $analyzer_constraint"

# sed -i is not portable, so we write to a separate file: https://unix.stackexchange.com/a/92907
dart tool/set_analyzer_constraint.dart "$analyzer_constraint"

pub get
robbecker-wf marked this conversation as resolved.
Show resolved Hide resolved

dart analyze .

# Verify the over_react builder runs without errors, since the builder itself isn't fully covered by tests.
dart run build_runner build --build-filter='**.dart' --delete-conflicting-outputs

dart run test -p vm -- test/vm_tests/builder
2 changes: 1 addition & 1 deletion tools/analyzer_plugin/playground/pubspec.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ dependencies:
over_react: ^3.5.3
dev_dependencies:
build_runner: '>=1.0.0 <3.0.0'
build_web_compilers: ^2.0.0
build_web_compilers: '>=2.0.0 <4.0.0'
workiva_analysis_options: ^1.1.0

dependency_overrides:
Expand Down