Fix parse errors with --preview-dart-2
#1874
Conversation
We need the latest analyzer, front_end, and async to get past static errors in these packages. Shelf needs to be updated to get the latest async. Fix a static error that is not caught by the analyzer around reassigning the final `stackTrace` variable in a catch clause.
lib/src/http.dart
Outdated
| @@ -68,7 +68,7 @@ class _PubHttpClient extends http.BaseClient { | |||
| streamedResponse = await _inner.send(request); | |||
| } on SocketException catch (error, stackTrace) { | |||
| // Work around issue 23008. | |||
| if (stackTrace == null) stackTrace = new Chain.current(); | |||
| final defaultedStackTrace = stackTrace ?? new Chain.current(); | |||
There was a problem hiding this comment.
Nit: var
I'd call the caught stack trace stackTraceOrNull and call this stackTrace, since this is used more frequently.
| @@ -3,9 +3,9 @@ name: pub | |||
| dependencies: | |||
| # Note: Pub's test infrastructure assumes that any dependencies used in tests | |||
| # will be hosted dependencies. | |||
| analyzer: ">=0.25.0 <0.31.0" | |||
| analyzer: ^0.31.2-alpha.1 | |||
There was a problem hiding this comment.
Are these tighter constraints really necessary? I generally try to keep constraints matching the actual APIs that are being used and let the version solver sort out which versions of the package are compatible with my Dart VM.
There was a problem hiding this comment.
Would you prefer ">=0.31.2-alpha.1 <0.32.0"?
There was a problem hiding this comment.
There are ZERO cases we care about where the analyzer version != the version in the Dart SDK.
Let's do everything we can to make sure the version ran on CI and dev machines matches that...
There was a problem hiding this comment.
@kevmoo The same argument could be made for every version constraint here, but setting a policy of constantly revving them to match the SDK's versions is untenable. We have thorough integration tests in the SDK to make sure the versions work out, and otherwise we should treat this like any other package and only muck with version constraints when it's actually necessary.
There was a problem hiding this comment.
synced offline. I think that "works with Dart 2" is a new behavior for the package that we want to start depending on, and I think it makes sense for our pub constraint to reflect that.
In the end, as we discussed, for this package it's mostly academic.
We need the latest analyzer, front_end, and async to get past static
errors in these packages. Shelf needs to be updated to get the latest
async.
Fix a static error that is not caught by the analyzer around reassigning
the final
stackTracevariable in a catch clause.