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 strict null checking #26

Merged
merged 1 commit into from
Oct 10, 2016
Merged

Enable strict null checking #26

merged 1 commit into from
Oct 10, 2016

Conversation

BurtHarris
Copy link
Collaborator

@BurtHarris BurtHarris commented Oct 5, 2016

Closes #14. I think you'll like this Sam.

Note this version generates some errors (of the form error TS2531: Object is possibly 'null') all of which seem valid to me. If we adopt this, it seems like it could replace the @NotNull decorator. (All errors are non-fatal, the tests still run.)

Seems like if we want to use this, the sooner the better.

@BurtHarris
Copy link
Collaborator Author

BurtHarris commented Oct 5, 2016

Note: after merging the latest changes from master, this generates one more interesting error, that shows a possible null return that I think is worth review/discussion.

src/misc/Interval.ts(159,7): error TS2322: Type 'null' is not assignable to type 'Interval'.

I don't understand from the comments if null is an expected return type for this function.

@BurtHarris
Copy link
Collaborator Author

I'm leaving the interesting errors in place for discussion. They do not block code generation or test success.

@BurtHarris BurtHarris changed the title Enabled strict null checking Enable strict null checking Oct 7, 2016
@BurtHarris
Copy link
Collaborator Author

BurtHarris commented Oct 8, 2016

Hmm, perhaps because I merged out-of-order, but right now this generates two implicit any errors:

src/dfa/ArrayEdgeMap.ts(150,4): error TS7017: Index signature of object type implicitly has an 'any' type.
src/dfa/SparseEdgeMap.ts(185,4): error TS7017: Index signature of object type implicitly has an 'any' type.

My last checkin fixed these.

@@ -125,7 +125,10 @@ class IterableAdapter<T> implements Iterable<T>, IterableIterator<T> {
[Symbol.iterator]() { this._iterator = this.collection.iterator(); return this;}

next(): IteratorResult<T> {
if (!this._iterator.hasNext()) return { done: true, value: undefined };
if (!this._iterator.hasNext()) {
// A bit of a hack needed here, tracking under https://github.com/Microsoft/TypeScript/issues/11375
Copy link
Member

Choose a reason for hiding this comment

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

@BurtHarris
Copy link
Collaborator Author

I think you said you'd handle this. P.S. Perhaps we should eliminate the Nullable() function from stubs.ts when we wrap this up.

Note this version generates some errors, all of which seem valid to me.  If we adopt this, it seems like it could replace the @NotNull decorator.
@sharwell sharwell merged commit 6242f41 into master Oct 10, 2016
@sharwell sharwell deleted the strictnull branch October 10, 2016 21:21
sharwell added a commit that referenced this pull request Oct 10, 2016
This change is based on work originally completed by @BurtHarris in #26.
sharwell added a commit that referenced this pull request Oct 10, 2016
This work is based on changes by @BurtHarris in #26.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants