Use type_comments when parsing AST

[KotlinIsland]

ast.parse doesn't parse type comments by default. This PR enables this option.

reference

[JelleZijlstra]

What observable behavior does this change? I'm worried it'll just lead to more crashes with an inconsistent AST (though maybe those are crashes we want to happen).

[KotlinIsland]

Not 100 percent sure, but when python is >= 3.8 it's behaving differently.

black/src/black/__init__.py

Lines 6468 to 6473 in 8e2b7b0

	# TypeIgnore has only one field 'lineno' which breaks this comparison
	type_ignore_classes = (ast3.TypeIgnore, ast27.TypeIgnore)
	if sys.version_info >= (3, 8):
	type_ignore_classes += (ast.TypeIgnore,)
	if isinstance(node, type_ignore_classes):
	break

3. _Black_ might move comments around, which includes type comments. Those are part of
   the AST as of Python 3.8. While the tool implements a number of special cases for
   those comments, there is no guarantee they will remain where they were in the source.
   Note that this doesn't change runtime behavior of the source code.

[JelleZijlstra]

That makes it look like this PR just makes us parse type comments we're ignoring anyway.

[KotlinIsland]

If type comments are ignored, why does python 3 - 3.7 use typed-ast? Why not just use normal ast?

I'm assuming that typed-ast is required for parsing python2 code.

[KotlinIsland]

If type comments are ignored then we should refactor the code that uses typed_ast for Python3 to use ast module instead.

[felix-hilden]

One such refactoring of the parsing code is currently going on in #2304. Got to say, I'm not so sure why typed_ast is needed either. I was under the impression that it was because of changes in the builtin module in 3.8, like using ast.Constant and the type comments. But when looking at typed_ast, they at least advertise only parsing type comments, so then us ignoring them is a bit weird. But I'm no expert on this.

[ichard26]

So okay, looks like this PR is stuck.

If type comments are ignored, why does python 3 - 3.7 use typed-ast? Why not just use normal ast?

Because unfortunately the built-in ast module didn't support feature_version until 3.8:

Changed in version 3.8: Added type_comments, mode='func_type' and feature_version.

https://docs.python.org/3/library/ast.html#ast.parse

I'm assuming that typed-ast is required for parsing python2 code.

True, but also needed below 3.8 to maintain compatibility with more grammars than the one of the runtime.

So unless the safety checks stop ignoring TypeIgnore instances, I don't think there's anything actionable here. Although I don't have much context on why we ignore those TypeIgnore in the first place.

[felix-hilden]

What observable behavior does this change? I'm worried it'll just lead to more crashes with an inconsistent AST (though maybe those are crashes we want to happen).

I think if we want to be safer with type comments, this could be an option. But then again, we have no such guarantees for other special comments. I'm quite indifferent about this, but personally now that the reasons for using typed vs. builtin ast are laid out, I'd rather have nice-looking code than correctly-typed code. Especially now that there is much more support for other types of annotations than comments. And brainstorming: maybe we could warn when we move a type comment 🤔

[JelleZijlstra]

Sounds like there is no clear rationale for this change, so I'm going to default to not changing anything.