Skip to content

Add type annotations #648

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

Merged
merged 1 commit into from
Feb 16, 2023
Merged

Add type annotations #648

merged 1 commit into from
Feb 16, 2023

Conversation

dubinsky
Copy link
Contributor

@dubinsky dubinsky commented Feb 7, 2023

Added type annotations for vals, vars and defs that did not have them; primary motivation: clarity.
Types inferred by IDEs like IntelliJ Idea are not always the types inferred by the Scala compiler.
In fact, different versions of the Scala compiler infer types differently!
As a result, in around 20 places it is impossible to add missing type annotations without breaking binary compatibility...

@dubinsky dubinsky closed this Feb 7, 2023
@dubinsky dubinsky reopened this Feb 7, 2023
@dubinsky dubinsky changed the title - add missing type annotations Add type annotations Feb 8, 2023
@dubinsky dubinsky marked this pull request as ready for review February 8, 2023 20:43
@dubinsky dubinsky merged commit 0ccb010 into scala:main Feb 16, 2023
@SethTisue
Copy link
Member

This goes farther than I would personally go in a codebase I worked heavily on; something like e.g. def foo: Boolean = false seems like noise to me versus def foo = false. And a lot of the changes are to local variables, which I'm far less likely to want type annotations on than I am in method signatures.

Regardless, I don't actually object to this being merged if you think it's a win overall.

Oh, heh, you hit "merge" right as I was writing this comment :-)

@dubinsky
Copy link
Contributor Author

This goes farther than I would personally go in a codebase I worked heavily on; something like e.g. def foo: Boolean = false seems like noise to me versus def foo = false. And a lot of the changes are to local variables, which I'm far less likely to want type annotations on than I am in method signatures.

Regardless, I don't actually object to this being merged if you think it's a win overall.

Oh, heh, you hit "merge" right as I was writing this comment :-)

Oops :(

You are right, some of the type annotations I added are trivial, but I wanted everything explicit because in some cases what I think the type is, what the IDE thinks and what the compiler thinks are all different...

@SethTisue
Copy link
Member

It's fine, suit yourself!

@dubinsky
Copy link
Contributor Author

Thanks!

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