dlang-community / dfmt

Dfmt is a formatter for D source code
Boost Software License 1.0
204 stars 47 forks source link

Single line functions #548

Open HuskyNator opened 2 years ago

HuskyNator commented 2 years ago

Requested enhancement to add the option of keeping single-line functions on one line. This is a feature in many other formatting tools & can improve readability when many tiny functions are included.

Current:

int returnTwo(){
   return 2;
}

Becomes:

int returnTwo(){ return 2; }
PetarKirov commented 2 years ago

One possibility would be to do that using using the arrow syntax: https://dlang.org/changelog/2.096.0.html#shortfunctions

The way this could work is that if dfmt detects that the function contains a single return <expr> statement it would automatically convert it to the new => <expr>; syntax. This would be an opt-in since we can't know if the -preview=shortenedMethods option is passed to the compiler. If the whole lines becomes to big it would be split like this:

ReturnType functionName(Args args) =>
    some.long.expression;
HuskyNator commented 2 years ago

I personally prefer not to use arrow syntax. (I'm honestly surprised the added support for them, they seem out of place to me)

rikkimax commented 2 years ago

I personally prefer not to use arrow syntax. (I'm honestly surprised the added support for them, they seem out of place to me)

The reason it was added was that it is already in use with lambdas. It closes a hole in the syntax to make it more consistent.

PetarKirov commented 2 years ago

they seem out of place to me

Well, the majority of mainstream languages nowadays disagrees :P

Honestly, I'd say D was the odd one out. As @rikkimax pointed out, that change makes the language more consistent and removes an artificial syntactic limitation.

Not quite syntactically equivalent, but semantically equivalent is the option to omit the the return statement if a function contains only a single return statement:

Functional languages (all functions are expressions):

WebFreak001 commented 2 years ago

dfmt never changes the source code tokens or does refactorings, it only manipulates whitespace. This has been the case for a long time, maybe even since start of the project, so other tools also might rely on it not manipulating any tokens. So I would strongly deny the formatter changing short return functions to =>, even with an option.

A refactoring would better fit in dfix or serve-d, where other refactorings are already implemented.

I agree with this issue that there should be an option to keep short methods short.

I would be for adding an extra option so that one-line methods are always kept one-line, even if they break the line length limit. D-Scanner or other tools can then complain if it's longer than the configured max line length and let the user fix it. I think that way would be better as otherwise if you had

int displayWidth() const @property { return _displayWidth; }
int displayHeight() const @property { return _displayHeight; }

and the line length limit would be just after the first line and before the second line, it would result in the first function staying one-line and the second one being split across multiple lines, which is ugly imo.

HuskyNator commented 2 years ago

Well, the majority of mainstream languages nowadays disagrees :P

That's alright. I'm all for syntactic freedom :)

It kind of reminds me of Java's improvements to switch syntax. (yield & ->) I wish D had this, or pattern matching (like match in scala, heck it seems even C# has it now)

HuskyNator commented 2 years ago

The same online addition may potentially be interesting for switch statement as well when single line expressions are used, Especially as dlang also has final switch statements (making this more prevalent).

Example:

switch (soor) {
case "SCALAR":
    return 1;
case "VEC2":
    return 2;
case "VEC3":
    return 3;
case "VEC4":
    return 4;
case "MAT2":
    return 4;
case "MAT3":
    return 9;
case "MAT4":
    return 16;
default:
    assert(0, "Unsupported accessors.type: " + type);
}

Compared to

switch (soor) {
case "SCALAR": return 1;
case "VEC2": return 2;
case "VEC3": return 3;
case "VEC4": return 4;
case "MAT2": return 4;
case "MAT3": return 9;
case "MAT4": return 16;
default: assert(0, "Unsupported accessors.type: " + type);
}

Clang also has this functionality as a nice example. image

PetarKirov commented 2 years ago

Agreed, and more generally, perhaps all AllowShort* clang format options are good candidates for dfmt.

AryzenRS commented 5 months ago

Additionally, I'd like to add statements like if (true) foo(); and if (true) { foo(); bar(); } to be allowed by dfmt, if that's acceptable...