diff --git a/design-docs/unified-configuration-and-task-model.md b/design-docs/unified-configuration-and-task-model.md index e589247d55d1..46dda198b63e 100644 --- a/design-docs/unified-configuration-and-task-model.md +++ b/design-docs/unified-configuration-and-task-model.md @@ -410,7 +410,7 @@ A new API for querying applied plugins that supports both `Plugin` implementing - ~~`Plugin` impl can include nested rule source class~~ - ~~A useful error message is presented to the user if they try to apply a rule source plugin as a regular plugin, i. e. `apply plugin: RuleSourcePlugin` or `apply { plugin RuleSourcePlugin }`~~ - Can use `PluginRegistry` and ids to check if both `Plugin` implementing classes and rule source classes are applied to a project -- A useful error message is presented when using `PluginContainer.withId()` or `PluginContainer.withType()` to check if a rule source plugin is applied +- ~~A useful error message is presented when using `PluginContainer.withId()` or `PluginContainer.withType()` to check if a rule source plugin is applied~~ ## Story: Model DSL rule uses a typed model element as input via name diff --git a/subprojects/core/src/main/groovy/org/gradle/api/internal/plugins/DefaultAppliedPlugins.java b/subprojects/core/src/main/groovy/org/gradle/api/internal/plugins/DefaultAppliedPlugins.java index 29d9b7c253bb..9b7e11255725 100644 --- a/subprojects/core/src/main/groovy/org/gradle/api/internal/plugins/DefaultAppliedPlugins.java +++ b/subprojects/core/src/main/groovy/org/gradle/api/internal/plugins/DefaultAppliedPlugins.java @@ -32,7 +32,7 @@ public DefaultAppliedPlugins(PluginAware target, PluginRegistry pluginRegistry) public void extractModelRulesAndAdd(Class pluginClass) { ModelRuleSourceDetector detector = new ModelRuleSourceDetector(); - if (detector.getDeclaredSources(pluginClass).size() > 0) { + if (!detector.getDeclaredSources(pluginClass).isEmpty()) { throw new UnsupportedOperationException(String.format("Cannot apply model rules of plugin '%s' as the target '%s' is not model rule aware", pluginClass.getName(), target)); } } diff --git a/subprojects/core/src/main/groovy/org/gradle/api/internal/plugins/DefaultObjectConfigurationAction.java b/subprojects/core/src/main/groovy/org/gradle/api/internal/plugins/DefaultObjectConfigurationAction.java index 31941acc4458..3c8fdedf4906 100755 --- a/subprojects/core/src/main/groovy/org/gradle/api/internal/plugins/DefaultObjectConfigurationAction.java +++ b/subprojects/core/src/main/groovy/org/gradle/api/internal/plugins/DefaultObjectConfigurationAction.java @@ -105,7 +105,7 @@ private void applyScript(Object script) { } private void applyPlugin(Class pluginClass) { - if (!Plugin.class.isAssignableFrom(pluginClass) && new ModelRuleSourceDetector().getDeclaredSources(pluginClass).size() > 0) { + if (!Plugin.class.isAssignableFrom(pluginClass) && !new ModelRuleSourceDetector().getDeclaredSources(pluginClass).isEmpty()) { throw new IllegalArgumentException(String.format("'%s' is a rule source only type, use 'type' key instead of 'plugin' key to apply it via PluginAware.apply()", pluginClass.getName())); } for (Object target : targets) { diff --git a/subprojects/core/src/main/groovy/org/gradle/api/internal/plugins/DefaultPluginCollection.java b/subprojects/core/src/main/groovy/org/gradle/api/internal/plugins/DefaultPluginCollection.java index 416d3bcae48b..856c7e366439 100644 --- a/subprojects/core/src/main/groovy/org/gradle/api/internal/plugins/DefaultPluginCollection.java +++ b/subprojects/core/src/main/groovy/org/gradle/api/internal/plugins/DefaultPluginCollection.java @@ -23,6 +23,7 @@ import org.gradle.api.plugins.PluginCollection; import org.gradle.api.specs.Spec; import org.gradle.api.specs.Specs; +import org.gradle.model.internal.inspect.ModelRuleSourceDetector; public class DefaultPluginCollection extends DefaultDomainObjectSet implements PluginCollection { @@ -39,6 +40,10 @@ protected DefaultPluginCollection filtered(CollectionFilter } public PluginCollection withType(Class type) { + if (!Plugin.class.isAssignableFrom(type) && !new ModelRuleSourceDetector().getDeclaredSources(type).isEmpty()) { + String message = String.format("'%s' is a rule source and not a plugin. Use AppliedPlugins.withPlugin() to perform an action if a rule source is applied.", type.getName()); + throw new IllegalArgumentException(message); + } return filtered(createFilter(type)); } diff --git a/subprojects/core/src/main/groovy/org/gradle/api/internal/plugins/DefaultPluginContainer.java b/subprojects/core/src/main/groovy/org/gradle/api/internal/plugins/DefaultPluginContainer.java index f378d53c3247..2b5071db2af2 100644 --- a/subprojects/core/src/main/groovy/org/gradle/api/internal/plugins/DefaultPluginContainer.java +++ b/subprojects/core/src/main/groovy/org/gradle/api/internal/plugins/DefaultPluginContainer.java @@ -24,6 +24,7 @@ import org.gradle.api.plugins.UnknownPluginException; import org.gradle.api.specs.Spec; import org.gradle.internal.UncheckedException; +import org.gradle.model.internal.inspect.ModelRuleSourceDetector; import java.util.Collections; import java.util.List; @@ -99,7 +100,7 @@ public DefaultPluginContainer(PluginRegistry pluginRegistry, T pluginAware, List } public Plugin apply(String id) { - return addPluginInternal(getTypeForId(id)); + return addPluginInternal(getPluginTypeForId(id)); } public

P apply(Class

type) { @@ -116,7 +117,7 @@ public boolean hasPlugin(Class type) { public Plugin findPlugin(String id) { try { - return findPlugin(getTypeForId(id)); + return findPlugin(getPluginTypeForId(id)); } catch (UnknownPluginException e) { return null; } @@ -167,6 +168,15 @@ public

P getPlugin(Class

type) throws UnknownPluginExcepti } public void withId(final String pluginId, Action action) { + try { + Class typeForId = pluginRegistry.getTypeForId(pluginId); + if (!Plugin.class.isAssignableFrom(typeForId) && !new ModelRuleSourceDetector().getDeclaredSources(typeForId).isEmpty()) { + String message = String.format("The type for id '%s' (class: '%s') is a rule source and not a plugin. Use AppliedPlugins.withPlugin() to perform an action if a rule source is applied.", pluginId, typeForId.getName()); + throw new IllegalArgumentException(message); + } + } catch (UnknownPluginException e) { + //just continue if it's impossible to see if the id points to a plain rule source + } matching(new Spec() { public boolean isSatisfiedBy(Plugin element) { try { @@ -178,7 +188,7 @@ public boolean isSatisfiedBy(Plugin element) { }).all(action); } - protected Class getTypeForId(String id) { + protected Class getPluginTypeForId(String id) { return pluginRegistry.getPluginTypeForId(id); } diff --git a/subprojects/core/src/main/groovy/org/gradle/api/internal/plugins/DefaultPluginRegistry.java b/subprojects/core/src/main/groovy/org/gradle/api/internal/plugins/DefaultPluginRegistry.java index fc02e9a36d90..a19febc8ae84 100644 --- a/subprojects/core/src/main/groovy/org/gradle/api/internal/plugins/DefaultPluginRegistry.java +++ b/subprojects/core/src/main/groovy/org/gradle/api/internal/plugins/DefaultPluginRegistry.java @@ -134,7 +134,7 @@ public Class getTypeForId(String pluginId) { return getTypeForId(pluginId, new Spec>() { public boolean isSatisfiedBy(Class rawClass) { ModelRuleSourceDetector detector = new ModelRuleSourceDetector(); - return Plugin.class.isAssignableFrom(rawClass) || detector.getDeclaredSources(rawClass).size() > 0; + return Plugin.class.isAssignableFrom(rawClass) || !detector.getDeclaredSources(rawClass).isEmpty(); } }, "Implementation class '%s' specified for plugin '%s' does not implement the Plugin interface and does not define any rule sources."); } diff --git a/subprojects/core/src/test/groovy/org/gradle/api/internal/plugins/DefaultPluginContainerTest.groovy b/subprojects/core/src/test/groovy/org/gradle/api/internal/plugins/DefaultPluginContainerTest.groovy index 7e744b0dbe9d..7020f8bd47e7 100644 --- a/subprojects/core/src/test/groovy/org/gradle/api/internal/plugins/DefaultPluginContainerTest.groovy +++ b/subprojects/core/src/test/groovy/org/gradle/api/internal/plugins/DefaultPluginContainerTest.groovy @@ -19,6 +19,7 @@ import org.gradle.api.Plugin import org.gradle.api.Project import org.gradle.api.internal.project.TestPlugin1 import org.gradle.api.internal.project.TestPlugin2 +import org.gradle.api.internal.project.TestRuleSource import org.gradle.api.plugins.UnknownPluginException import org.gradle.internal.reflect.DirectInstantiator import org.gradle.test.fixtures.file.TestNameTestDirectoryProvider @@ -109,6 +110,7 @@ public class DefaultPluginContainerTest extends Specification { def "executes action for plugin with given id"() { def plugin = new TestPlugin1() + pluginRegistry.getTypeForId("plugin") >> TestPlugin1 pluginRegistry.getPluginTypeForId("plugin") >> TestPlugin1 def plugins = [] container.add(plugin) @@ -256,4 +258,25 @@ public class DefaultPluginContainerTest extends Specification { 1 * applicationAction1.execute({ it.plugin == plugin && it.target == project }) 1 * applicationAction2.execute({ it.plugin == plugin && it.target == project }) } + + def "a useful error message is set when a plain rule source type is passed to withType"() { + when: + container.withType(TestRuleSource) + + then: + IllegalArgumentException e = thrown() + e.message == "'$TestRuleSource.name' is a rule source and not a plugin. Use AppliedPlugins.withPlugin() to perform an action if a rule source is applied." + } + + def "a useful error message is set when an id for plain rule source type is passed to withId"() { + given: + pluginRegistry.getTypeForId("custom-rule-source") >> TestRuleSource + + when: + container.withId("custom-rule-source") {} + + then: + IllegalArgumentException e = thrown() + e.message == "The type for id 'custom-rule-source' (class: '$TestRuleSource.name') is a rule source and not a plugin. Use AppliedPlugins.withPlugin() to perform an action if a rule source is applied." + } } diff --git a/subprojects/model-core/src/test/groovy/org/gradle/model/internal/inspect/ModelRuleSourceDetectorTest.groovy b/subprojects/model-core/src/test/groovy/org/gradle/model/internal/inspect/ModelRuleSourceDetectorTest.groovy index 03d4e508b2e5..05cede8f86f8 100644 --- a/subprojects/model-core/src/test/groovy/org/gradle/model/internal/inspect/ModelRuleSourceDetectorTest.groovy +++ b/subprojects/model-core/src/test/groovy/org/gradle/model/internal/inspect/ModelRuleSourceDetectorTest.groovy @@ -22,6 +22,8 @@ import spock.lang.Unroll class ModelRuleSourceDetectorTest extends Specification { + private ModelRuleSourceDetector detector = new ModelRuleSourceDetector() + static class HasOneSource { @RuleSource static class Source {} @@ -46,7 +48,7 @@ class ModelRuleSourceDetectorTest extends Specification { @Unroll def "find model rule sources - #clazz"() { expect: - new ModelRuleSourceDetector().getDeclaredSources(clazz) == expected.toSet() + detector.getDeclaredSources(clazz) == expected.toSet() where: clazz | expected