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

Don't always inline expression lambdas #147

Merged
merged 18 commits into from
Jan 29, 2020
Merged

Conversation

dansanduleac
Copy link
Contributor

@dansanduleac dansanduleac commented Jan 23, 2020

Before this PR

We always allow an inlining to stop at a lambda, regardless of the complexity of the expression following the lambda.

This can sometimes lead to inlining a bunch of arguments but breaking up a super simple lambda, which is not readable at all:

publisher = new ThisThatEventPublisher(
        txnManager, () -> Events.encoder(keyPair, new TestClock()), EventSeries.THING, () ->
                DEFAULT_EVENT_BATCH_SIZE);

After this PR

==COMMIT_MSG==
More strictly control when we can inline a chunk of code followed by an expression lambda.

Specifically, the inlining chain now continues through to the body of the expression lambda, with the exception that the inlined expression so far is reset to be considered "simple" when entering a lambda expression.

The exception is necessary in order to preserve the inlining of palantir-chains-lambdas which we consider desirable.
==COMMIT_MSG==

Example changes on:

Possible downsides?

  • Inadvertently changing more code that looked better before.
    I think this is acceptable as long as the existing code doesn't become less readable but just a bit more sparse, if in return we get rid of some corner cases that were extremely unreadable / confusing.

@changelog-app
Copy link

changelog-app bot commented Jan 23, 2020

Generate changelog in changelog/@unreleased

Type

  • Feature
  • Improvement
  • Fix
  • Break
  • Deprecation
  • Manual task
  • Migration

Description

More strictly control when we can inline a chunk of code followed by an expression lambda, to avoid pathological cases with a very simple expression being split onto the next line.

Check the box to generate changelog(s)

  • Generate changelog entry

@dansanduleac dansanduleac changed the title Ds/better lambdas Don't always inline expression lambdas Jan 23, 2020
@policy-bot policy-bot bot requested a review from iamdanfox January 23, 2020 17:58
@dansanduleac
Copy link
Contributor Author

Holding off on this for a bit until we can determine how many changes it will cause in a large internal repo.

# Conflicts:
#	palantir-java-format/src/main/java/com/palantir/javaformat/doc/Level.java
@bulldozer-bot bulldozer-bot bot merged commit cc783a8 into develop Jan 29, 2020
@bulldozer-bot bulldozer-bot bot deleted the ds/better-lambdas branch January 29, 2020 18:18
@svc-autorelease
Copy link
Collaborator

Released 0.3.19

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants