Skip to content

refactor: directly expose static final String fields / remove getters#706

Merged
johnrengelman merged 1 commit intoGradleUp:masterfrom
Fiouz:replace_properties_with_fields
Oct 4, 2021
Merged

refactor: directly expose static final String fields / remove getters#706
johnrengelman merged 1 commit intoGradleUp:masterfrom
Fiouz:replace_properties_with_fields

Conversation

@Fiouz
Copy link
Copy Markdown
Contributor

@Fiouz Fiouz commented Aug 30, 2021

Omitting the visibility modifier creates a Groovy property [1], which
is composed by:

  • a private backing field
  • a public getter
  • (+ setter if the property is not final)

It is unlikely to be an intended design as it makes access to those
fields awkward from statically typed languages
ShadowBasePlugin.getCONFIGURATION_NAME().

This commit replaces such properties with public fields only.

This is a backward incompatible change as this removes public getters.

Here is what the generated class looked liked before this
commit (7.0.0):

$ javap -p -c -constants ShadowBasePlugin.class
Compiled from "ShadowBasePlugin.groovy"
public class com.github.jengelman.gradle.plugins.shadow.ShadowBasePlugin implements org.gradle.api.Plugin<org.gradle.api.Project>, groovy.lang.GroovyObject {
  private static final java.lang.String EXTENSION_NAME = "shadow";

  private static final java.lang.String CONFIGURATION_NAME = "shadow";
[...]
  public static java.lang.String getEXTENSION_NAME();
    Code:
       0: getstatic     #86                 // Field EXTENSION_NAME:Ljava/lang/String;
       3: areturn

  public static java.lang.String getCONFIGURATION_NAME();
    Code:
       0: getstatic     #111                // Field CONFIGURATION_NAME:Ljava/lang/String;
       3: areturn

[1] https://groovy-lang.org/objectorientation.html#properties

Omitting the visibility modifier creates a Groovy _property_ [1], which
is composed by:
* a _private_ backing field
* a public getter
* (+ setter if the property is not `final`)

It is unlikely to be an intended design as it makes access to those
fields awkward from statically typed languages
`ShadowBasePlugin.getCONFIGURATION_NAME()`.

This commit replaces such _properties_ with _public fields_ only.

This is a backward incompatible change as this removes public getters.

Here is what the generated class looked liked before this
commit (7.0.0):

```
$ javap -p -c -constants ShadowBasePlugin.class
Compiled from "ShadowBasePlugin.groovy"
public class com.github.jengelman.gradle.plugins.shadow.ShadowBasePlugin implements org.gradle.api.Plugin<org.gradle.api.Project>, groovy.lang.GroovyObject {
  private static final java.lang.String EXTENSION_NAME = "shadow";

  private static final java.lang.String CONFIGURATION_NAME = "shadow";
[...]
  public static java.lang.String getEXTENSION_NAME();
    Code:
       0: getstatic     GradleUp#86                 // Field EXTENSION_NAME:Ljava/lang/String;
       3: areturn

  public static java.lang.String getCONFIGURATION_NAME();
    Code:
       0: getstatic     GradleUp#111                // Field CONFIGURATION_NAME:Ljava/lang/String;
       3: areturn
```

[1] https://groovy-lang.org/objectorientation.html#properties
@johnrengelman johnrengelman added this to the 7.1 milestone Oct 4, 2021
@johnrengelman johnrengelman merged commit c0d21cf into GradleUp:master Oct 4, 2021
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