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

Enable null safety #472

Merged
merged 1 commit into from
Sep 29, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
36 changes: 24 additions & 12 deletions .travis.yml
Original file line number Diff line number Diff line change
@@ -1,23 +1,35 @@
language: dart

dart:
- dev
- 2.4.0
- dev

dart_task:
- test: --platform vm,chrome
- dartanalyzer: --fatal-infos --fatal-warnings .

matrix:
jobs:
include:
# Only validate formatting using the dev release
- dart: dev
dart_task: dartfmt
- stage: analyze_and_format
name: "Analyze"
os: linux
script: dartanalyzer --enable-experiment=non-nullable --fatal-warnings --fatal-infos .
- stage: analyze_and_format
name: "Format"
os: linux
script: dartfmt -n --set-exit-if-changed .
- stage: test
name: "Vm Tests"
os: linux
script: pub run --enable-experiment=non-nullable test -p vm
- stage: test
name: "Web Tests"
os: linux
script: pub run --enable-experiment=non-nullable test -p chrome

stages:
- analyze_and_format
- test

# Only building master means that we don't run two builds for each pull request.
branches:
only: [master]

cache:
directories:
- $HOME/.pub-cache
directories:
- $HOME/.pub-cache
13 changes: 13 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,16 @@
## 0.13.0-nullsafety-dev

Pre-release for the null safety migration of this package.

Note that 0.12.3 may not be the final stable null safety release version, we
reserve the right to release it as a 0.13.0 breaking change.

This release will be pinned to only allow pre-release sdk versions starting from
2.10.0-2.0.dev, which is the first version where this package will appear in the
null safety allow list.

- Added `const` constructor to `ByteStream`.

## 0.12.2

* Fix error handler callback type for response stream errors to avoid masking
Expand Down
4 changes: 4 additions & 0 deletions analysis_options.yaml
Original file line number Diff line number Diff line change
@@ -1,7 +1,11 @@
include: package:pedantic/analysis_options.yaml

analyzer:
strong-mode:
implicit-casts: false
enable-experiment:
- non-nullable

linter:
rules:
- annotate_overrides
Expand Down
22 changes: 11 additions & 11 deletions lib/http.dart
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ export 'src/streamed_response.dart';
/// the same server, you should use a single [Client] for all of those requests.
///
/// For more fine-grained control over the request, use [Request] instead.
Future<Response> head(url, {Map<String, String> headers}) =>
Future<Response> head(Object url, {Map<String, String>? headers}) =>
_withClient((client) => client.head(url, headers: headers));

/// Sends an HTTP GET request with the given headers to the given URL, which can
Expand All @@ -41,7 +41,7 @@ Future<Response> head(url, {Map<String, String> headers}) =>
/// the same server, you should use a single [Client] for all of those requests.
///
/// For more fine-grained control over the request, use [Request] instead.
Future<Response> get(url, {Map<String, String> headers}) =>
Future<Response> get(Object url, {Map<String, String>? headers}) =>
_withClient((client) => client.get(url, headers: headers));

/// Sends an HTTP POST request with the given headers and body to the given URL,
Expand All @@ -63,8 +63,8 @@ Future<Response> get(url, {Map<String, String> headers}) =>
///
/// For more fine-grained control over the request, use [Request] or
/// [StreamedRequest] instead.
Future<Response> post(url,
{Map<String, String> headers, body, Encoding encoding}) =>
Future<Response> post(Object url,
{Map<String, String>? headers, Object? body, Encoding? encoding}) =>
_withClient((client) =>
client.post(url, headers: headers, body: body, encoding: encoding));

Expand All @@ -87,8 +87,8 @@ Future<Response> post(url,
///
/// For more fine-grained control over the request, use [Request] or
/// [StreamedRequest] instead.
Future<Response> put(url,
{Map<String, String> headers, body, Encoding encoding}) =>
Future<Response> put(Object url,
{Map<String, String>? headers, Object? body, Encoding? encoding}) =>
_withClient((client) =>
client.put(url, headers: headers, body: body, encoding: encoding));

Expand All @@ -111,8 +111,8 @@ Future<Response> put(url,
///
/// For more fine-grained control over the request, use [Request] or
/// [StreamedRequest] instead.
Future<Response> patch(url,
{Map<String, String> headers, body, Encoding encoding}) =>
Future<Response> patch(Object url,
{Map<String, String>? headers, Object? body, Encoding? encoding}) =>
_withClient((client) =>
client.patch(url, headers: headers, body: body, encoding: encoding));

Expand All @@ -124,7 +124,7 @@ Future<Response> patch(url,
/// the same server, you should use a single [Client] for all of those requests.
///
/// For more fine-grained control over the request, use [Request] instead.
Future<Response> delete(url, {Map<String, String> headers}) =>
Future<Response> delete(Object url, {Map<String, String>? headers}) =>
_withClient((client) => client.delete(url, headers: headers));

/// Sends an HTTP GET request with the given headers to the given URL, which can
Expand All @@ -140,7 +140,7 @@ Future<Response> delete(url, {Map<String, String> headers}) =>
///
/// For more fine-grained control over the request and response, use [Request]
/// instead.
Future<String> read(url, {Map<String, String> headers}) =>
Future<String> read(Object url, {Map<String, String>? headers}) =>
_withClient((client) => client.read(url, headers: headers));

/// Sends an HTTP GET request with the given headers to the given URL, which can
Expand All @@ -156,7 +156,7 @@ Future<String> read(url, {Map<String, String> headers}) =>
///
/// For more fine-grained control over the request and response, use [Request]
/// instead.
Future<Uint8List> readBytes(url, {Map<String, String> headers}) =>
Future<Uint8List> readBytes(Object url, {Map<String, String>? headers}) =>
_withClient((client) => client.readBytes(url, headers: headers));

Future<T> _withClient<T>(Future<T> Function(Client) fn) async {
Expand Down
28 changes: 15 additions & 13 deletions lib/src/base_client.dart
Original file line number Diff line number Diff line change
Expand Up @@ -18,40 +18,42 @@ import 'streamed_response.dart';
/// maybe [close], and then they get various convenience methods for free.
abstract class BaseClient implements Client {
@override
Future<Response> head(url, {Map<String, String> headers}) =>
Future<Response> head(Object url, {Map<String, String>? headers}) =>
_sendUnstreamed('HEAD', url, headers);

@override
Future<Response> get(url, {Map<String, String> headers}) =>
Future<Response> get(Object url, {Map<String, String>? headers}) =>
_sendUnstreamed('GET', url, headers);

@override
Future<Response> post(url,
{Map<String, String> headers, body, Encoding encoding}) =>
Future<Response> post(Object url,
{Map<String, String>? headers, Object? body, Encoding? encoding}) =>
_sendUnstreamed('POST', url, headers, body, encoding);

@override
Future<Response> put(url,
{Map<String, String> headers, body, Encoding encoding}) =>
Future<Response> put(Object url,
{Map<String, String>? headers, Object? body, Encoding? encoding}) =>
_sendUnstreamed('PUT', url, headers, body, encoding);

@override
Future<Response> patch(url,
{Map<String, String> headers, body, Encoding encoding}) =>
Future<Response> patch(Object url,
{Map<String, String>? headers, Object? body, Encoding? encoding}) =>
_sendUnstreamed('PATCH', url, headers, body, encoding);

@override
Future<Response> delete(url, {Map<String, String> headers}) =>
Future<Response> delete(Object url, {Map<String, String>? headers}) =>
_sendUnstreamed('DELETE', url, headers);

@override
Future<String> read(url, {Map<String, String> headers}) async {
Future<String> read(Object url, {Map<String, String>? headers}) async {
final response = await get(url, headers: headers);
_checkResponseSuccess(url, response);
return response.body;
}

@override
Future<Uint8List> readBytes(url, {Map<String, String> headers}) async {
Future<Uint8List> readBytes(Object url,
{Map<String, String>? headers}) async {
final response = await get(url, headers: headers);
_checkResponseSuccess(url, response);
return response.bodyBytes;
Expand All @@ -69,8 +71,8 @@ abstract class BaseClient implements Client {

/// Sends a non-streaming [Request] and returns a non-streaming [Response].
Future<Response> _sendUnstreamed(
String method, url, Map<String, String> headers,
[body, Encoding encoding]) async {
String method, url, Map<String, String>? headers,
[body, Encoding? encoding]) async {
var request = Request(method, _fromUriOrString(url));

if (headers != null) request.headers.addAll(headers);
Expand Down
13 changes: 8 additions & 5 deletions lib/src/base_request.dart
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@

import 'dart:collection';

import 'package:meta/meta.dart';

import 'byte_stream.dart';
import 'client.dart';
import 'streamed_response.dart';
Expand All @@ -29,10 +31,10 @@ abstract class BaseRequest {
///
/// This defaults to `null`, which indicates that the size of the request is
/// not known in advance. May not be assigned a negative value.
int get contentLength => _contentLength;
int _contentLength;
int? get contentLength => _contentLength;
int? _contentLength;

set contentLength(int value) {
set contentLength(int? value) {
jakemac53 marked this conversation as resolved.
Show resolved Hide resolved
if (value != null && value < 0) {
throw ArgumentError('Invalid content length $value.');
}
Expand Down Expand Up @@ -93,16 +95,17 @@ abstract class BaseRequest {
/// Freezes all mutable fields and returns a single-subscription [ByteStream]
/// that emits the body of the request.
///
/// The base implementation of this returns null rather than a [ByteStream];
/// The base implementation of this returns an empty [ByteStream];
/// subclasses are responsible for creating the return value, which should be
/// single-subscription to ensure that no data is dropped. They should also
/// freeze any additional mutable fields they add that don't make sense to
/// change after the request headers are sent.
@mustCallSuper
ByteStream finalize() {
// TODO(nweiz): freeze headers
if (finalized) throw StateError("Can't finalize a finalized Request.");
_finalized = true;
return null;
return const ByteStream(Stream.empty());
}

/// Sends this request.
Expand Down
8 changes: 4 additions & 4 deletions lib/src/base_response.dart
Original file line number Diff line number Diff line change
Expand Up @@ -10,18 +10,18 @@ import 'base_request.dart';
/// they're returned by [BaseClient.send] or other HTTP client methods.
abstract class BaseResponse {
/// The (frozen) request that triggered this response.
final BaseRequest request;
final BaseRequest? request;

/// The HTTP status code for this response.
final int statusCode;

/// The reason phrase associated with the status code.
final String reasonPhrase;
final String? reasonPhrase;

/// The size of the response body, in bytes.
///
/// If the size of the request is not known in advance, this is `null`.
final int contentLength;
final int? contentLength;

// TODO(nweiz): automatically parse cookies from headers

Expand All @@ -42,7 +42,7 @@ abstract class BaseResponse {
this.reasonPhrase}) {
if (statusCode < 100) {
throw ArgumentError('Invalid status code $statusCode.');
} else if (contentLength != null && contentLength < 0) {
} else if (contentLength != null && contentLength! < 0) {
throw ArgumentError('Invalid content length $contentLength.');
}
}
Expand Down
11 changes: 7 additions & 4 deletions lib/src/browser_client.dart
Original file line number Diff line number Diff line change
Expand Up @@ -52,16 +52,17 @@ class BrowserClient extends BaseClient {
request.headers.forEach(xhr.setRequestHeader);

var completer = Completer<StreamedResponse>();

// TODO(kevmoo): Waiting on https://github.com/dart-lang/linter/issues/2185
// ignore: void_checks
Copy link
Member Author

Choose a reason for hiding this comment

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

Here and below – not sure why we're getting this void_checks hint...

Copy link
Member

Choose a reason for hiding this comment

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

This will be worked around by googlearchive/pedantic#69 if we get that in.

This issue looks like https://github.com/dart-lang/linter/issues/2185

I think we can keep the lint and point the todo at that issue.

unawaited(xhr.onLoad.first.then((_) {
// TODO(nweiz): Set the response type to "arraybuffer" when issue 18542
Copy link
Member Author

Choose a reason for hiding this comment

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

This issue is now closed

// is fixed.
var blob = xhr.response as Blob ?? Blob([]);
var blob = xhr.response as Blob;
Copy link
Member Author

Choose a reason for hiding this comment

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

According to the annotations on the dart:html types, response is never null.

Copy link
Member

Choose a reason for hiding this comment

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

What do you mean by this? I think it probably is true that if onLoad.first doesn't have an error this would be non-null, but I don't know what you mean by "the annotations on the dart:html types".

var reader = FileReader();

reader.onLoad.first.then((_) {
var body = reader.result as Uint8List;
completer.complete(StreamedResponse(
ByteStream.fromBytes(body), xhr.status,
ByteStream.fromBytes(body), xhr.status!,
contentLength: body.length,
request: request,
headers: xhr.responseHeaders,
Expand All @@ -76,6 +77,8 @@ class BrowserClient extends BaseClient {
reader.readAsArrayBuffer(blob);
}));

// TODO(kevmoo): Waiting on https://github.com/dart-lang/linter/issues/2185
// ignore: void_checks
unawaited(xhr.onError.first.then((_) {
// Unfortunately, the underlying XMLHttpRequest API doesn't expose any
// specific information about the error itself.
Expand Down
2 changes: 1 addition & 1 deletion lib/src/byte_stream.dart
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import 'dart:typed_data';

/// A stream of chunks of bytes representing a single piece of data.
class ByteStream extends StreamView<List<int>> {
ByteStream(Stream<List<int>> stream) : super(stream);
const ByteStream(Stream<List<int>> stream) : super(stream);

/// Returns a single-subscription byte stream that will emit the given bytes
/// in a single chunk.
Expand Down
Loading