-
Notifications
You must be signed in to change notification settings - Fork 591
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
[C#] support class definitions without a body #4120
base: master
Are you sure you want to change the base?
Conversation
- include: terminator | ||
- match: (?=\S) | ||
pop: true | ||
pop: 1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not important, but you use pop: true
for the rest of the commit (except the pop: 2
).
/// ^^^^^^^^^^^^^^^^^^^^^^^ meta.class.cs | ||
/// ^ punctuation.separator.type.cs | ||
/// ^^^^^^^^^ entity.other.inherited-class.cs | ||
/// ^^^^^^^^^^ meta.function-call.cs meta.group.cs |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
meta.function-call
feels off somehow in combination with inherited-class.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But logically we want it scoped like a normal function call I believe, a constructor is just a special type of function 🙂 I think other constructor calls are like that, though I'm not at a computer to check right now. Like new SomeClass("this is a constructor call")
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I read the line as a declaration, which forwards arguments from specialized InvalidException
class to Exception
base class in case of an instantiation. The line doesn't actually call any constructors, does it?
The actual instantiation, which calls a constructor as part of object creation would be new InvalidException("Error message")
, but that's not the topic here.
Even if, I wouldn't scope an object instantiation "meta.function-call" either, as calling one of the available constructors is only one part of the whole object creation process.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What scope can we use here then? :)
btw in the below InvalidException
and InvalidException2
are basically identical, just using different syntax.
public class InvalidException(string _message) : Exception(_message);
public class InvalidException2 : Exception
{
public InvalidException2(string _message) : base(_message)
{
}
}
and if we had a breakpoint inside the constructor for Exception
, it would get hit when calling new InvalidException("Error message")
, which is why I felt it can be just treated as another function call...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, those constructors are called on new InvalidException()
. The actual function-call is invoked implicitly when instantiating via new
operator.
Both, long and short form, however just denote a class definition. See how also base(_message)
is not scoped meta.function-call
in current C# syntax, because it does not belong to constructor's normal function body but just denote's which parameters to pass to super class's constructor.
I find this special syntax worth dedicated scopes, especially as they describe a sort of inheritance or dependency tree.
We can find such constructs in Python dataclasses as well as Java's record class
definitions:
Packages/Java/tests/syntax_test_java.java
Lines 2527 to 2536 in c7cd5a7
record RecordTest<T>(int x, @notnull foo bar) implements Foo, Bar { } | |
// <- meta.class.java keyword.declaration.record.java | |
//^^^^ meta.class.java keyword.declaration.record.java | |
// ^^^^^^^^^^^^^^ meta.class.identifier.java | |
// ^^^ meta.generic.declaration.java | |
// ^^^^^^^^^^^^^^^^^^^^^^^^^ meta.class.parameters.java meta.group.java | |
// ^^^^^^^^ meta.annotation.identifier.java | |
// ^ meta.class.java | |
// ^^^^^^^^^^^^^^^^^^^^ meta.class.implements.java | |
// ^^^ meta.class.java meta.block.java |
... except none of them supports direct parameter forwarding to parent classes in declaration syntax.
IIRC, Java is the only syntax which applies special meta.class.[extends|implements]
meta scopes to the inherited : Exception
part without however needing to address any argument list.
We could go with it and just scope those argument lists meta.group
....
public class InvalidException(string _message) : Exception(_message);
// ^^^^^^^^^^^^^^^^ meta.class.identifier entity.name.class
// ^^^^^^^^^^^^^^^^ meta.class.parameters meta.group
// ^^^ meta.class
// ^^^^^^^^^^^^^^^^^^^^ meta.class.[extends|inherites]
// ^^^^^^^^^ entity.other.inherited-class
// ^^^^^^^^^^ meta.group
or add a new meta.class.extends.[identifier|arguments]
sub-scopes, covering whole Exception(_message)
, like ...
public class InvalidException(string _message) : Exception(_message);
// ^^^^^^^^^^^^^^^^ meta.class.identifier entity.name.class
// ^^^^^^^^^^^^^^^^ meta.class.parameters meta.group
// ^^^ meta.class
// ^ meta.class.extends
// ^^^^^^^^^ meta.class.extends.identifier entity.other.inherited-class
// ^^^^^^^^^^ meta.class.extends.arguments meta.group
or just stack another meta.class.[identifier|arguments]
like...
public class InvalidException(string _message) : Exception(_message);
// ^^^^^^^^^^^^^^^^ meta.class.identifier entity.name.class
// ^^^^^^^^^^^^^^^^ meta.class.parameters meta.group
// ^^^ meta.class
// ^^^^^^^^^^^^^^^^^^^^ meta.class.extends
// ^^^^^^^^^ meta.class.identifier entity.other.inherited-class
// ^^^^^^^^^^ meta.class.arguments meta.group
Note: Not sure if it is correct, but I somehow got used to use arguments
if parens describe arguments being passed to and parameters
if parens denote a real signature definition.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FWIW, Java scopes instantiations meta.instantiaton
Packages/Java/tests/syntax_test_java.java
Lines 7973 to 7978 in c7cd5a7
new @Foo TestClass(foo); | |
// ^^^^^^^^^^^^^^^^^^^^^^^ meta.instantiation.java | |
// ^^^^ meta.annotation.identifier.java | |
// ^^^^^^^^^^^^^^^ - meta.annotation | |
// ^^^^^ meta.group.java | |
// ^^ - meta.instantiation |
It distinguishes it from normal function calls to illustrate it being a special syntax to create a new object and implicitly calling its construtors to initialize attributes/members.
I could also imagine to add sub scopes for identifiers and argument lists like meta.instantiation.[identifier|arguments]
.
fixes #4119
plus converts some anonymous contexts into named contexts