Skip to content

refactor: convert log calls to parameterized logging in DDK builder/generator/UI #1307

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.

Sign up for GitHub

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

Merged
joaodinissf merged 1 commit into from
May 18, 2026
Merged

refactor: convert log calls to parameterized logging in DDK builder/generator/UI #1307

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
3 changes: 2 additions & 1 deletion com.avaloq.tools.ddk.xtext.builder/META-INF/MANIFEST.MF
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,8 @@ Require-Bundle: org.eclipse.xtext.builder,
com.avaloq.tools.ddk.xtext,
org.eclipse.xtext.common.types.ui
Bundle-RequiredExecutionEnvironment: JavaSE-21
Import-Package: org.apache.logging.log4j, org.apache.log4j
Import-Package: org.apache.logging.log4j,
org.apache.logging.log4j.util
Export-Package: com.avaloq.tools.ddk.xtext.builder,
com.avaloq.tools.ddk.xtext.builder.layered,
com.avaloq.tools.ddk.xtext.builder.resourceloader,
Expand Down
4 changes: 1 addition & 3 deletions ...ext.builder/src/com/avaloq/tools/ddk/xtext/builder/CachingStateBasedContainerManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -122,9 +122,7 @@ public void descriptionsChanged(final Event event) {
public List<IContainer> getVisibleContainers(final IResourceDescription desc, final IResourceDescriptions resourceDescriptions) {
String root = internalGetContainerHandle(desc, resourceDescriptions);
if (root == null) {
if (LOGGER.isDebugEnabled()) {
LOGGER.debug("Cannot find IContainer for: " + desc.getURI()); //$NON-NLS-1$
}
LOGGER.debug("Cannot find IContainer for: {}", desc::getURI); //$NON-NLS-1$
return Collections.emptyList();
}
List<String> handles = getState(resourceDescriptions).getVisibleContainerHandles(root);
Expand Down
16 changes: 8 additions & 8 deletions ...xtext.builder/src/com/avaloq/tools/ddk/xtext/builder/MonitoredClusteringBuilderState.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ public class MonitoredClusteringBuilderState extends ClusteringBuilderState

private static final int COMMIT_WARN_WAIT_SEC = 30;

private static final String FAILED_TO_SAVE_BINARY = "Failed to save binary for "; //$NON-NLS-1$
private static final String FAILED_TO_SAVE_BINARY_LOG = "Failed to save binary for {}"; //$NON-NLS-1$

/** Class-wide logger. */
private static final Logger LOGGER = LogManager.getLogger(MonitoredClusteringBuilderState.class);
Expand Down Expand Up @@ -642,7 +642,7 @@ public boolean isCanceled() {
final long memoryDelta = Runtime.getRuntime().freeMemory() - initialMemory;
final int resourceSetSizeDelta = resourceSet.getResources().size() - initialResourceSetSize;
final long timeDelta = System.nanoTime() - initialTime;
traceSet.trace(ResourceLinkingMemoryEvent.class, changedURI, memoryDelta, resourceSetSizeDelta, timeDelta);
traceSet.trace(ResourceLinkingMemoryEvent.class, changedURI, memoryDelta, resourceSetSizeDelta, timeDelta); // NOPMD GuardLogStatement - not a logger call
}
watchdog.reportWorkEnded(index, index + queue.size());
}
Expand Down Expand Up @@ -727,9 +727,9 @@ protected void storeBinaryResource(final Resource resource, final BuildData buil
} catch (RejectedExecutionException e) {
String errorMessage = "Unable to submit a new task to store a binary resource."; //$NON-NLS-1$
if (binaryStorageExecutor.isShutdown()) {
LOGGER.info(errorMessage + " The worker pool has shut down."); //$NON-NLS-1$
LOGGER.info("{} The worker pool has shut down.", errorMessage); //$NON-NLS-1$
} else {
LOGGER.error(errorMessage + " Exception information: " + e.getMessage()); //$NON-NLS-1$
LOGGER.error("{} Exception information: {}", errorMessage, e.getMessage()); //$NON-NLS-1$
}
}
}
Expand All @@ -751,12 +751,12 @@ protected void doStoreBinaryResource(final Resource resource, final BuildData bu
LOGGER.info("saving binary taking longer than expected ({} ms): {}", elapsedMillis, resource.getURI()); //$NON-NLS-1$
}
} catch (WrappedException ex) {
LOGGER.error(FAILED_TO_SAVE_BINARY + resource.getURI(), ex.exception());
LOGGER.error(FAILED_TO_SAVE_BINARY_LOG, resource.getURI(), ex.exception());

// CHECKSTYLE:OFF
} catch (Throwable ex) {
// CHECKSTYLE:ON
LOGGER.error(FAILED_TO_SAVE_BINARY + resource.getURI(), ex);
LOGGER.error(FAILED_TO_SAVE_BINARY_LOG, resource.getURI(), ex);
}
}

Expand Down Expand Up @@ -1205,7 +1205,7 @@ protected void flushChanges(final ResourceDescriptionsData newData) {
}
long flushTime = TimeUnit.MILLISECONDS.toSeconds(System.currentTimeMillis() - time);
if (flushTime > COMMIT_WARN_WAIT_SEC) {
LOGGER.warn("Flushing of the database changes took " + flushTime + " seconds."); //$NON-NLS-1$//$NON-NLS-2$
LOGGER.warn("Flushing of the database changes took {} seconds.", flushTime); //$NON-NLS-1$
}
}
}
Expand Down Expand Up @@ -1299,7 +1299,7 @@ protected void queueAffectedResources(final Set<URI> allRemainingURIs, final IRe
// CHECKSTYLE:CHECK-OFF IllegalCatch - Failing here means the build fails completely
} catch (Throwable t) {
// CHECKSTYLE:CHECK-ON IllegalCatch
LOGGER.warn(manager.getClass().getSimpleName() + " failed to enqueue the affected resources", t); //$NON-NLS-1$
LOGGER.warn("{} failed to enqueue the affected resources", manager.getClass().getSimpleName(), t); //$NON-NLS-1$
}
progressMonitor.worked(1);
if (allRemainingURIs.isEmpty()) {
Expand Down
15 changes: 6 additions & 9 deletions ...ools.ddk.xtext.builder/src/com/avaloq/tools/ddk/xtext/builder/RebuildingXtextBuilder.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -161,10 +161,9 @@ protected IProject[] build(final int kind, final Map args, final IProgressMonito
@Override
protected void doBuild(final ToBeBuilt toBeBuilt, final IProgressMonitor monitor, final BuildType type) throws CoreException {
if (!toBeBuilt.getToBeDeleted().isEmpty() || !toBeBuilt.getToBeUpdated().isEmpty()) {
if (LOGGER.isDebugEnabled()) {
LOGGER.debug("Starting " + type.name() + " build:" + "\ndeleted(" + toBeBuilt.getToBeDeleted().size() + ")=" + toBeBuilt.getToBeDeleted().toString()
+ "\nupdated(" + toBeBuilt.getToBeUpdated().size() + ")=" + toBeBuilt.getToBeUpdated().toString());
}
LOGGER.debug("Starting {} build:\ndeleted({})={}\nupdated({})={}", type::name, // NOPMD GuardLogStatement - all args are Suppliers
Copy link
Copy Markdown
Member

@rubenporras rubenporras May 18, 2026

Choose a reason for hiding this comment

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

this is not quite correct, we are calling getters here, however, I do not think it matters much in this case because these getters do not do object allocation.

Copy link
Copy Markdown
Collaborator Author

@joaodinissf joaodinissf May 18, 2026

Choose a reason for hiding this comment

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

Good catch, that's right. Going ahead since these are lightweight.

toBeBuilt.getToBeDeleted()::size, toBeBuilt.getToBeDeleted()::toString,
toBeBuilt.getToBeUpdated()::size, toBeBuilt.getToBeUpdated()::toString);

SubMonitor progress = SubMonitor.convert(monitor, 1 + 1 + 1); // build + participant + rebuild

Expand Down Expand Up @@ -194,11 +193,9 @@ protected void doBuild(final ToBeBuilt toBeBuilt, final IProgressMonitor monitor
}
resourceSet.getResources().clear();
resourceSet.eAdapters().clear();
if (LOGGER.isDebugEnabled()) {
LOGGER.debug("Build done.");
}
} else if (LOGGER.isDebugEnabled()) {
LOGGER.debug("Ignoring empty " + type.name() + " build.");
LOGGER.debug("Build done.");
} else {
LOGGER.debug("Ignoring empty {} build.", type::name);
}
}
}
6 changes: 3 additions & 3 deletions ...xt.expression/src/com/avaloq/tools/ddk/xtext/expression/generator/CompilationContext.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -223,7 +223,7 @@ public boolean isType(final String name) {
public String javaType(final String name) {
Type type = findType(name);
if (type == null) {
LOGGER.warn("No type found for " + name);
LOGGER.warn("No type found for {}", name);
return name;
}
return javaType(type);
Expand Down Expand Up @@ -320,7 +320,7 @@ public String getQualifiedTypeName(final String typeName) {
}
// CHECKSTYLE:OFF
} catch (final Exception e) {
LOGGER.warn(NLS.bind("Could not determine qualified type name for {0}", typeName), e); //$NON-NLS-1$
LOGGER.warn("Could not determine qualified type name for {}", typeName, e); //$NON-NLS-1$
}
// CHECKSTYLE:ON

Expand All @@ -343,7 +343,7 @@ public EClass getEClass(final Type type) {
// CHECKSTYLE:OFF
} catch (Exception e) {
// CHECKSTYLE:ON
LOGGER.error("Could not determine EClass for " + type, e);
LOGGER.error("Could not determine EClass for {}", type, e);
}
}
return null;
Expand Down
2 changes: 1 addition & 1 deletion ...k.xtext.expression/src/com/avaloq/tools/ddk/xtext/expression/generator/GenModelUtil2.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,7 @@ public static GenModel findGenModel(final EModelElement eModelElement, final Res
// CHECKSTYLE:CHECK-OFF IllegalCatch
} catch (RuntimeException e) {
// CHECKSTYLE:CHECK-ON IllegalCatch
LOGGER.error(NLS.bind("Exception in findGenModel ({0})", eModelElement), e); //$NON-NLS-1$
LOGGER.error("Exception in findGenModel ({})", eModelElement, e); //$NON-NLS-1$
}

return null;
Expand Down
3 changes: 2 additions & 1 deletion com.avaloq.tools.ddk.xtext.generator/META-INF/MANIFEST.MF
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,8 @@ Require-Bundle: org.eclipse.jface,
org.eclipse.jdt.core,
org.eclipse.compare,
org.eclipse.xtext.xbase
Import-Package: org.apache.logging.log4j
Import-Package: org.apache.logging.log4j,
org.apache.logging.log4j.util
Export-Package: com.avaloq.tools.ddk.xtext.generator.builder,
com.avaloq.tools.ddk.xtext.generator.ecore,
com.avaloq.tools.ddk.xtext.generator.expression,
Expand Down
4 changes: 2 additions & 2 deletions ...enerator/src/com/avaloq/tools/ddk/xtext/generator/parser/antlr/KeywordAnalysisHelper.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -254,7 +254,7 @@ public void printViolations(final String srcGenPath) {
}
if (problemsReported) {
LOGGER.error("REJECTED KEYWORDS BY ID RULES DETECTED.");
LOGGER.error("Read " + fileName + " !");
LOGGER.error("Read {} !", fileName);
writer.println();
writer.println("(!) Problems were detected: neither reserved words nor keywords, but rejected by identifier rules");
writer.println("Use " + Path.fromPortableString(getReportFileName(srcGenPath)).lastSegment() + " to find out why these words are keywords.");
Expand Down Expand Up @@ -448,7 +448,7 @@ public void printReport(final String srcGenPath) {
PrintWriter docuWriter = new PrintWriter(new File(docuFileName), StandardCharsets.UTF_8);
docuWriter.print(new CombinedGrammarReportBuilder(grammarExtensions).getDocumentation(grammar, parserRules, enumRules));
docuWriter.close();
LOGGER.info("report on keywords is written into " + fileName);
LOGGER.info("report on keywords is written into {}", fileName);
} catch (IOException e) {
throw new UncheckedIOException(e);
}
Expand Down
7 changes: 3 additions & 4 deletions ...nerator/src/com/avaloq/tools/ddk/xtext/generator/util/CustomClassAwareEcoreGenerator.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ public JControlModel getJControlModel() {
return jControlModel;
}
};
LOGGER.info("generating EMF code for " + this.genModel);
LOGGER.info("generating EMF code for {}", this.genModel);
generator.getAdapterFactoryDescriptorRegistry().addDescriptor(GenModelPackage.eNS_URI, new GeneratorAdapterDescriptor(getTypeMapper(), getLineDelimiter()));
generator.setInput(model);

Expand Down Expand Up @@ -170,7 +170,7 @@ public void preInvoke() {
}
}
}
LOGGER.info("Registered source path to discover custom classes: " + Joiner.on(", ").join(this.srcPaths));
LOGGER.info("Registered source path to discover custom classes: {}", () -> Joiner.on(", ").join(this.srcPaths));
}

/**
Expand All @@ -187,8 +187,7 @@ private void addSourcePathForPlugin(final String pluginId) {
}
URI createURI = URI.createURI(path);
if (!URIConverter.INSTANCE.exists(createURI, null)) {
LOGGER.warn("Cannot access path '" + path
+ "'. Make sure to checkout all parent languages before running the workflow. Otherwise custom implementations (*ImplCustom) will not be considered.");
LOGGER.warn("Cannot access path '{}'. Make sure to checkout all parent languages before running the workflow. Otherwise custom implementations (*ImplCustom) will not be considered.", path);
}
this.srcPaths.add(path);
}
Expand Down
3 changes: 2 additions & 1 deletion com.avaloq.tools.ddk.xtext.ui/META-INF/MANIFEST.MF
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,8 @@ Require-Bundle: com.avaloq.tools.ddk.xtext,
com.google.guava,
com.avaloq.tools.ddk.xtext.ide,
com.avaloq.tools.ddk
Import-Package: org.apache.logging.log4j
Import-Package: org.apache.logging.log4j,
org.apache.logging.log4j.util
Export-Package: com.avaloq.tools.ddk.xtext.ui,
com.avaloq.tools.ddk.xtext.ui.editor,
com.avaloq.tools.ddk.xtext.ui.editor.findrefs,
Expand Down
6 changes: 1 addition & 5 deletions ...xt.ui/src/com/avaloq/tools/ddk/xtext/ui/editor/model/ResponsiveXtextDocumentProvider.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@
import org.eclipse.jface.preference.IPreferenceStore;
import org.eclipse.jface.util.IPropertyChangeListener;
import org.eclipse.jface.util.PropertyChangeEvent;
import org.eclipse.osgi.util.NLS;
import org.eclipse.ui.preferences.ScopedPreferenceStore;
import org.eclipse.xtext.Constants;
import org.eclipse.xtext.ui.editor.model.XtextDocument;
Expand Down Expand Up @@ -81,11 +80,8 @@ protected void disconnected() {
* the event
*/
@Override
@SuppressWarnings("PMD.UnnecessaryVarargsArrayCreation")
public void propertyChange(final PropertyChangeEvent event) {
if (LOGGER.isDebugEnabled()) {
LOGGER.debug(NLS.bind("Preference Change: {0} => {1} -> {2}", new Object[] {event.getProperty(), event.getOldValue(), event.getNewValue()})); //$NON-NLS-1$
}
LOGGER.debug("Preference Change: {} => {} -> {}", event::getProperty, event::getOldValue, event::getNewValue); //$NON-NLS-1$

for (Iterator<?> i = getConnectedElements(); i.hasNext();) {
((XtextDocument) getDocument(i.next())).checkAndUpdateAnnotations();
Expand Down
2 changes: 1 addition & 1 deletion com.avaloq.tools.ddk.xtext.ui/src/com/avaloq/tools/ddk/xtext/ui/util/RuntimeProjectUtil.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ public boolean apply(final Pair<IStorage, IProject> input) {

return (IFile) fileStorage.getFirst();
} catch (NoSuchElementException e) {
LOGGER.debug("Cannot find file storage for " + uri); //$NON-NLS-1$
LOGGER.debug("Cannot find file storage for {}", uri); //$NON-NLS-1$
return null;
}
}
Expand Down