-
Notifications
You must be signed in to change notification settings - Fork 1
Add Gradle plugin rules for lazy configuration, service injection, and extension best practices #31
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
Conversation
…d extension best practices
…peConstants class
| rules.put("task project access", taskActionShouldNotAccessProject); | ||
| rules.put("task dependencies", taskActionShouldNotCallGetTaskDependencies); | ||
| rules.put("lazy task registration", LAZY_TASK_CREATION); | ||
| rules.put("use named instead of getByName", USE_NAMED_INSTEAD_OF_GET_BY_NAME); |
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.
🎉
| */ | ||
| public static final ArchRule EXTENSION_PROPERTIES_USE_PROVIDER_API = ArchRuleDefinition.priority(Priority.MEDIUM) | ||
| .classes() | ||
| .that().haveSimpleNameEndingWith("Extension") |
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.
this seems kind of handwavy... maybe there is a way to find classes that are called as an arg to ExtensionsContainer.create?
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 was thinking about that. Need to dig deeper but wondering if I can look at the plugin class that creates the extension and then find the extension code but that seems like crossing boundaries in an archrule from what I understand so far, I'll think more about this one
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.
actually,
I think I'll use getAccessesToSelf and see if it is accessed from a plugin. I think that will add an extra level of certainty
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.
94d68cf improves things a bit.
I don't think I can detect project.extensions.create("greeting", GreetingExtension::class.java) easily 🤔
| * system and improves build performance. | ||
| */ | ||
| public static final ArchRule EXTENSION_PROPERTIES_USE_PROVIDER_API = ArchRuleDefinition.priority(Priority.MEDIUM) | ||
| .classes() |
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.
this condition should probably be more in the shape of "fields declared in classes that (logic) should use provider api" and then the same for methods()
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.
addressed by 66252c2
| */ | ||
| public static final ArchRule PLUGINS_SHOULD_NOT_STORE_PROJECT_REFERENCES = ArchRuleDefinition.priority(Priority.HIGH) | ||
| .classes() | ||
| .that().implement("org.gradle.api.Plugin") |
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.
this condition can be more like "fields declared in classes that implement plugin should not have type of Project"
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.
Done in 9cd1673
…ves from naming convention
…e plugin architecture rules
|
This is ready for review again @OdysseusLives @wakingrufus |
New Rules Added
Task Container API Rules
tasks.named()instead oftasks.getByName()configureEach()instead ofall()Service Injection Rules
ObjectFactoryvia constructor instead of callingproject.getObjects()ProviderFactoryvia constructor instead of callingproject.getProviders()Provider API Rules
Project Reference Rule
Refactoring
TypeConstants.javato consolidate 32+ duplicate constant declarations across 5 filesisProviderApiType(),callsMethodOn(),callsMethodOnAny()