Skip to content

Remove unused method parameters #25986

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

Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ private void configureAsciidoctorTask(Project project, AbstractAsciidoctorTask a
asciidoctorTask.baseDirFollowsSourceDir();
createSyncDocumentationSourceTask(project, asciidoctorTask);
if (asciidoctorTask instanceof AsciidoctorTask) {
configureAsciidoctorHtmlTask(project, (AsciidoctorTask) asciidoctorTask);
configureAsciidoctorHtmlTask((AsciidoctorTask) asciidoctorTask);
}
}

Expand Down Expand Up @@ -144,7 +144,7 @@ private Sync createSyncDocumentationSourceTask(Project project, AbstractAsciidoc
return syncDocumentationSource;
}

private void configureAsciidoctorHtmlTask(Project project, AsciidoctorTask asciidoctorTask) {
private void configureAsciidoctorHtmlTask(AsciidoctorTask asciidoctorTask) {
asciidoctorTask.outputOptions((outputOptions) -> outputOptions.backends("spring-html"));
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2012-2020 the original author or authors.
* Copyright 2012-2021 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand All @@ -22,6 +22,7 @@
import org.gradle.api.plugins.JavaPlugin;
import org.gradle.api.plugins.JavaPluginExtension;
import org.gradle.api.publish.PublishingExtension;
import org.gradle.api.publish.VariantVersionMappingStrategy;
import org.gradle.api.publish.maven.MavenPom;
import org.gradle.api.publish.maven.MavenPomDeveloperSpec;
import org.gradle.api.publish.maven.MavenPomIssueManagement;
Expand Down Expand Up @@ -78,8 +79,7 @@ void apply(Project project) {

private void customizeMavenPublication(MavenPublication publication, Project project) {
customizePom(publication.getPom(), project);
project.getPlugins().withType(JavaPlugin.class)
.all((javaPlugin) -> customizeJavaMavenPublication(publication, project));
project.getPlugins().withType(JavaPlugin.class).all((javaPlugin) -> customizeJavaMavenPublication(publication));
}

private void customizePom(MavenPom pom, Project project) {
Expand All @@ -97,11 +97,11 @@ private void customizePom(MavenPom pom, Project project) {
}
}

private void customizeJavaMavenPublication(MavenPublication publication, Project project) {
private void customizeJavaMavenPublication(MavenPublication publication) {
publication.versionMapping((strategy) -> strategy.usage(Usage.JAVA_API, (mappingStrategy) -> mappingStrategy
.fromResolutionOf(JavaPlugin.RUNTIME_CLASSPATH_CONFIGURATION_NAME)));
publication.versionMapping((strategy) -> strategy.usage(Usage.JAVA_RUNTIME,
(mappingStrategy) -> mappingStrategy.fromResolutionResult()));
publication.versionMapping(
(strategy) -> strategy.usage(Usage.JAVA_RUNTIME, VariantVersionMappingStrategy::fromResolutionResult));
}

private void customizeOrganization(MavenPomOrganization organization) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,6 @@
import org.springframework.context.annotation.DependsOn;
import org.springframework.core.env.Environment;
import org.springframework.core.io.Resource;
import org.springframework.core.io.ResourceLoader;
import org.springframework.core.type.AnnotatedTypeMetadata;
import org.springframework.jdbc.datasource.SimpleDriverDataSource;
import org.springframework.util.StringUtils;
Expand All @@ -63,8 +62,7 @@
@Deprecated
class DataSourceInitializationConfiguration {

private static DataSource determineDataSource(Supplier<DataSource> dataSource, String username, String password,
DataSourceProperties properties) {
private static DataSource determineDataSource(Supplier<DataSource> dataSource, String username, String password) {
if (StringUtils.hasText(username) && StringUtils.hasText(password)) {
DataSourceBuilder.derivedFrom(dataSource.get()).type(SimpleDriverDataSource.class).username(username)
.password(password).build();
Expand Down Expand Up @@ -92,29 +90,29 @@ static class InitializationSpecificCredentialsDataSourceInitializationConfigurat

@Bean
DataSourceScriptDatabaseInitializer ddlOnlyScriptDataSourceInitializer(ObjectProvider<DataSource> dataSource,
DataSourceProperties properties, ResourceLoader resourceLoader) {
DataSourceProperties properties) {
DatabaseInitializationSettings settings = new DatabaseInitializationSettings();
settings.setSchemaLocations(scriptLocations(properties.getSchema(), "schema", properties.getPlatform()));
settings.setContinueOnError(properties.isContinueOnError());
settings.setSeparator(properties.getSeparator());
settings.setEncoding(properties.getSqlScriptEncoding());
DataSource initializationDataSource = determineDataSource(dataSource::getObject,
properties.getSchemaUsername(), properties.getSchemaPassword(), properties);
properties.getSchemaUsername(), properties.getSchemaPassword());
return new InitializationModeDataSourceScriptDatabaseInitializer(initializationDataSource, settings,
properties.getInitializationMode());
}

@Bean
@DependsOn("ddlOnlyScriptDataSourceInitializer")
DataSourceScriptDatabaseInitializer dmlOnlyScriptDataSourceInitializer(ObjectProvider<DataSource> dataSource,
DataSourceProperties properties, ResourceLoader resourceLoader) {
DataSourceProperties properties) {
DatabaseInitializationSettings settings = new DatabaseInitializationSettings();
settings.setDataLocations(scriptLocations(properties.getData(), "data", properties.getPlatform()));
settings.setContinueOnError(properties.isContinueOnError());
settings.setSeparator(properties.getSeparator());
settings.setEncoding(properties.getSqlScriptEncoding());
DataSource initializationDataSource = determineDataSource(dataSource::getObject,
properties.getDataUsername(), properties.getDataPassword(), properties);
properties.getDataUsername(), properties.getDataPassword());
return new InitializationModeDataSourceScriptDatabaseInitializer(initializationDataSource, settings,
properties.getInitializationMode());
}
Expand Down Expand Up @@ -149,7 +147,7 @@ static class SharedCredentialsDataSourceInitializationConfiguration {

@Bean
DataSourceScriptDatabaseInitializer scriptDataSourceInitializer(DataSource dataSource,
DataSourceProperties properties, ResourceLoader resourceLoader) {
DataSourceProperties properties) {
DatabaseInitializationSettings settings = new DatabaseInitializationSettings();
settings.setSchemaLocations(scriptLocations(properties.getSchema(), "schema", properties.getPlatform()));
settings.setDataLocations(scriptLocations(properties.getData(), "data", properties.getPlatform()));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -217,16 +217,15 @@ private Set<BinderOption> asBinderOptionsSet(BinderOption... options) {
private Binder getBinder(ConfigDataActivationContext activationContext,
Predicate<ConfigDataEnvironmentContributor> filter, Set<BinderOption> options) {
boolean failOnInactiveSource = options.contains(BinderOption.FAIL_ON_BIND_TO_INACTIVE_SOURCE);
Iterable<ConfigurationPropertySource> sources = () -> getBinderSources(activationContext,
Iterable<ConfigurationPropertySource> sources = () -> getBinderSources(
filter.and((contributor) -> failOnInactiveSource || contributor.isActive(activationContext)));
PlaceholdersResolver placeholdersResolver = new ConfigDataEnvironmentContributorPlaceholdersResolver(this.root,
activationContext, failOnInactiveSource);
BindHandler bindHandler = !failOnInactiveSource ? null : new InactiveSourceChecker(activationContext);
return new Binder(sources, placeholdersResolver, null, null, bindHandler);
}

private Iterator<ConfigurationPropertySource> getBinderSources(ConfigDataActivationContext activationContext,
Predicate<ConfigDataEnvironmentContributor> filter) {
private Iterator<ConfigurationPropertySource> getBinderSources(Predicate<ConfigDataEnvironmentContributor> filter) {
return this.root.stream().filter(this::hasConfigurationPropertySource).filter(filter)
.map(ConfigDataEnvironmentContributor::getConfigurationPropertySource).iterator();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,11 +57,6 @@ class ConfigDataProperties {
* @param activate the activate properties
*/
ConfigDataProperties(@Name("import") List<ConfigDataLocation> imports, Activate activate) {
this(imports, activate, Collections.emptyList());
}

private ConfigDataProperties(List<ConfigDataLocation> imports, Activate activate,
List<ConfigurationProperty> boundProperties) {
this.imports = (imports != null) ? imports : Collections.emptyList();
this.activate = activate;
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2012-2020 the original author or authors.
* Copyright 2012-2021 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -52,15 +52,14 @@ public boolean isResolvable(ConfigDataLocationResolverContext context, ConfigDat
public List<ConfigTreeConfigDataResource> resolve(ConfigDataLocationResolverContext context,
ConfigDataLocation location) {
try {
return resolve(context, location.getNonPrefixedValue(PREFIX));
return resolve(location.getNonPrefixedValue(PREFIX));
}
catch (IOException ex) {
throw new ConfigDataLocationNotFoundException(location, ex);
}
}

private List<ConfigTreeConfigDataResource> resolve(ConfigDataLocationResolverContext context, String location)
throws IOException {
private List<ConfigTreeConfigDataResource> resolve(String location) throws IOException {
Assert.isTrue(location.endsWith("/"),
() -> String.format("Config tree location '%s' must end with '/'", location));
if (!this.resourceLoader.isPattern(location)) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -753,21 +753,21 @@ Elements append(Elements additional) {
ElementType[] type = new ElementType[size];
System.arraycopy(this.type, 0, type, 0, this.size);
System.arraycopy(additional.type, 0, type, this.size, additional.size);
CharSequence[] resolved = newResolved(0, size);
CharSequence[] resolved = newResolved(size);
for (int i = 0; i < additional.size; i++) {
resolved[this.size + i] = additional.get(i);
}
return new Elements(this.source, size, this.start, this.end, type, resolved);
}

Elements chop(int size) {
CharSequence[] resolved = newResolved(0, size);
CharSequence[] resolved = newResolved(size);
return new Elements(this.source, size, this.start, this.end, this.type, resolved);
}

Elements subElements(int offset) {
int size = this.size - offset;
CharSequence[] resolved = newResolved(offset, size);
CharSequence[] resolved = newResolved(size);
int[] start = new int[size];
System.arraycopy(this.start, offset, start, 0, size);
int[] end = new int[size];
Expand All @@ -777,7 +777,7 @@ Elements subElements(int offset) {
return new Elements(this.source, size, start, end, type, resolved);
}

private CharSequence[] newResolved(int offset, int size) {
private CharSequence[] newResolved(int size) {
CharSequence[] resolved = new CharSequence[size];
if (this.resolved != null) {
System.arraycopy(this.resolved, 0, resolved, 0, Math.min(size, this.size));
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2012-2020 the original author or authors.
* Copyright 2012-2021 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -86,10 +86,10 @@ public void onApplicationEvent(ApplicationEvent event) {
onApplicationEnvironmentPreparedEvent((ApplicationEnvironmentPreparedEvent) event);
}
if (event instanceof ApplicationPreparedEvent) {
onApplicationPreparedEvent((ApplicationPreparedEvent) event);
onApplicationPreparedEvent();
}
if (event instanceof ApplicationFailedEvent) {
onApplicationFailedEvent((ApplicationFailedEvent) event);
onApplicationFailedEvent();
}
}

Expand All @@ -101,11 +101,11 @@ private void onApplicationEnvironmentPreparedEvent(ApplicationEnvironmentPrepare
}
}

private void onApplicationPreparedEvent(ApplicationPreparedEvent event) {
private void onApplicationPreparedEvent() {
finish();
}

private void onApplicationFailedEvent(ApplicationFailedEvent event) {
private void onApplicationFailedEvent() {
finish();
}
Comment on lines +104 to 110
Copy link

@cuspymd cuspymd Apr 10, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems a bit ambiguous to call it a handler function for the event by deleting the event parameter.
Wouldn't it be better to get rid of the wrapper function and call finish() directly in onApplicationEvent()?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thought about that actually, but I liked the clear separation of things here. For me they are still the containers for the logic that should happen in case of the respective event and just share the common finish() call - regardless of the fact that they don't need the parameter. But I have no strong feelings for either option. Let's see what the team thinks and I'll gladly change to whatever is preferred.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I prefer it as currently proposed.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the considering.


Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2012-2020 the original author or authors.
* Copyright 2012-2021 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -29,7 +29,6 @@
import ch.qos.logback.core.util.OptionHelper;

import org.springframework.boot.logging.LogFile;
import org.springframework.boot.logging.LoggingInitializationContext;

/**
* Default logback configuration used by Spring Boot. Uses {@link LogbackConfigurator} to
Expand All @@ -46,7 +45,7 @@ class DefaultLogbackConfiguration {

private final LogFile logFile;

DefaultLogbackConfiguration(LoggingInitializationContext initializationContext, LogFile logFile) {
DefaultLogbackConfiguration(LogFile logFile) {
this.logFile = logFile;
}

Expand Down Expand Up @@ -106,13 +105,12 @@ private Appender<ILoggingEvent> fileAppender(LogbackConfigurator config, String
appender.setEncoder(encoder);
config.start(encoder);
appender.setFile(logFile);
setRollingPolicy(appender, config, logFile);
setRollingPolicy(appender, config);
config.appender("FILE", appender);
return appender;
}

private void setRollingPolicy(RollingFileAppender<ILoggingEvent> appender, LogbackConfigurator config,
String logFile) {
private void setRollingPolicy(RollingFileAppender<ILoggingEvent> appender, LogbackConfigurator config) {
SizeAndTimeBasedRollingPolicy<ILoggingEvent> rollingPolicy = new SizeAndTimeBasedRollingPolicy<>();
rollingPolicy.setContext(config.getContext());
rollingPolicy.setFileNamePattern(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,7 @@ protected void loadDefaults(LoggingInitializationContext initializationContext,
new LogbackLoggingSystemProperties(environment, context::putProperty).apply(logFile);
LogbackConfigurator configurator = debug ? new DebugLogbackConfigurator(context)
: new LogbackConfigurator(context);
new DefaultLogbackConfiguration(initializationContext, logFile).apply(configurator);
new DefaultLogbackConfiguration(logFile).apply(configurator);
context.setPackagingDataEnabled(true);
}

Expand Down