Skip to content
Merged
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 @@ -4,6 +4,7 @@
import java.util.Arrays;
import java.util.List;

import com.google.common.collect.Lists;
import com.google.gson.Gson;

public class CliMessager {
Expand All @@ -12,8 +13,17 @@ public class CliMessager {
// The END string lets us know when a message stops, which should prevent bugs involving multi-line output.
private static final String END = "SFDX-END";

private static final String REALTIME_START = "SFCA-REALTIME-START";
private static final String REALTIME_END = "SFCA-REALTIME-END";

/* Deprecated: Don't maintain state in a class that's essentially used as a utility.*/
@Deprecated
private static final List<Message> MESSAGES = new ArrayList<>();

/**
* Deprecated - switch to static invocation of {@link #postMessage(String, EventKey, String...)}
*/
@Deprecated
public static CliMessager getInstance() {
return LazyHolder.INSTANCE;
}
Expand All @@ -26,6 +36,7 @@ public static CliMessager getInstance() {
*
* @param exception to send to Typescript layer
*/
@Deprecated
public void addMessage(MessagePassableException exception) {
final EventKey eventKey = exception.getEventKey();
addMessage(
Expand All @@ -43,34 +54,59 @@ public void addMessage(MessagePassableException exception) {
* @param eventKey EventKey to display to user
* @param args String args passed to the EventKey to make the displayed message meaningful
*/
@Deprecated
public void addMessage(String internalLog, EventKey eventKey, String... args) {
// Developer error if eventKey was not added to exception and we'll get a bunch of NPEs
assert (eventKey != null);
// Confirm that the correct number of arguments for the message has been provided
// If this fails, this would be a developer error
assert (eventKey.getArgCount() == args.length);

final Message message = new Message(
eventKey.getMessageKey(),
Arrays.asList(args),
internalLog,
eventKey.getMessageType(),
eventKey.getMessageHandler(),
eventKey.isVerbose());
MESSAGES.add(message);
final Message message = createMessage(internalLog, eventKey, args);
MESSAGES.add(message);
}

/**
/**
* Publish formatted stdout message to pass onto Typescript layer.
* Make sure EventKey is updated with messages/EventKeyTemplates.json
* and has correct properties in the enum.
*
* @param internalLog Information for internal use. Will be logged but not displayed to user
* @param eventKey EventKey to display to user
* @param args String args passed to the EventKey to make the displayed message meaningful
*/
public static void postMessage(String internalLog, EventKey eventKey, String... args) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Creating an alternate method to post realtime messages. I've marked the other entry points as deprecated and we can eventually make them realtime as well.

final Message message = createMessage(internalLog, eventKey, args);
final List<Message> messages = Lists.newArrayList(message);

final String messageAsJson = new Gson().toJson(messages);
System.out.println(REALTIME_START + messageAsJson + REALTIME_END);
}

private static Message createMessage(String internalLog, EventKey eventKey, String[] args) {
// Developer error if eventKey was not added to exception and we'll get a bunch of NPEs
assert (eventKey != null);
// Confirm that the correct number of arguments for the message has been provided
// If this fails, this would be a developer error
assert (eventKey.getArgCount() == args.length);

final Message message = new Message(
eventKey.getMessageKey(),
Arrays.asList(args),
internalLog,
eventKey.getMessageType(),
eventKey.getMessageHandler(),
eventKey.isVerbose());
return message;
}

/**
* Convert all messages stored by the instance into a JSON-formatted string, enclosed in the start and end strings.
* Java code can use this method to log the messages to console, and TypeScript code can seek the start and stop
* strings to get an array of messages that can be deserialized.
* @return
*/
@Deprecated
public String getAllMessagesWithFormatting() {
final String messagesAsJson = getMessagesAsJson();
return START + messagesAsJson + END;
}

@Deprecated
private String getMessagesAsJson() {
return new Gson().toJson(MESSAGES);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@

public enum EventKey {
// MAKE SURE `messageKey` OF EVERY VALUE ADDED HERE HAS AN ENTRY IN 'messages/EventKeyTemplates.js'!

/** PMD-CATALOGER RELATED **/
INFO_GENERAL_INTERNAL_LOG("info.generalInternalLog", 1, MessageType.INFO, MessageHandler.INTERNAL, true),
WARNING_INVALID_CAT_SKIPPED("warning.invalidCategorySkipped", 1, MessageType.WARNING, MessageHandler.UX, true),
WARNING_INVALID_RULESET_SKIPPED("warning.invalidRulesetSkipped", 1, MessageType.WARNING, MessageHandler.UX, true),
Expand All @@ -20,8 +22,23 @@ public enum EventKey {
ERROR_EXTERNAL_RECURSION_LIMIT("error.external.recursionLimitReached", 2, MessageType.ERROR, MessageHandler.UX, false),
ERROR_EXTERNAL_XML_NOT_READABLE("error.external.xmlNotReadable", 2, MessageType.ERROR, MessageHandler.UX, false),
ERROR_EXTERNAL_XML_NOT_PARSABLE("error.external.xmlNotParsable", 2, MessageType.ERROR, MessageHandler.UX, false),




/** SFGE RELATED **/
Copy link
Contributor Author

Choose a reason for hiding this comment

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

New event keys to match the new messages

INFO_GENERAL("info.sfgeInfoLog", 1, MessageType.INFO, MessageHandler.UX, true),
INFO_META_INFO_COLLECTED("info.sfgeMetaInfoCollected", 2, MessageType.INFO, MessageHandler.UX, true),
INFO_COMPLETED_FILE_COMPILATION("info.sfgeFinishedCompilingFiles", 1, MessageType.INFO, MessageHandler.UX_SPINNER, false),
INFO_STARTED_BUILDING_GRAPH("info.sfgeStartedBuildingGraph", 0, MessageType.INFO, MessageHandler.UX_SPINNER, false),
INFO_COMPLETED_BUILDING_GRAPH("info.sfgeFinishedBuildingGraph", 0, MessageType.INFO, MessageHandler.UX_SPINNER, false),
INFO_PATH_ENTRY_POINTS_IDENTIFIED("info.sfgePathEntryPointsIdentified", 1, MessageType.INFO, MessageHandler.UX_SPINNER, false),
INFO_PATH_ANALYSIS_PROGRESS("info.sfgeViolationsInPathProgress", 4, MessageType.INFO, MessageHandler.UX_SPINNER, false),
INFO_COMPLETED_PATH_ANALYSIS("info.sfgeCompletedPathAnalysis", 3, MessageType.INFO, MessageHandler.UX_SPINNER, false),
WARNING_GENERAL("warning.sfgeWarnLog", 1, MessageType.WARNING, MessageHandler.UX, true),
WARNING_MULTIPLE_METHOD_TARGET_MATCHES("warning.multipleMethodTargetMatches", 3, MessageType.WARNING, MessageHandler.UX, false),
WARNING_NO_METHOD_TARGET_MATCHES("warning.noMethodTargetMatches", 2, MessageType.WARNING, MessageHandler.UX, false);
WARNING_NO_METHOD_TARGET_MATCHES("warning.noMethodTargetMatches", 2, MessageType.WARNING, MessageHandler.UX, false),
ERROR_GENERAL("error.internal.sfgeErrorLog", 1, MessageType.ERROR, MessageHandler.UX, false);

final String messageKey;
final int argCount;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
public class Message {
final private String messageKey;
final private List<String> args;
final private String internalLog;
final private String internalLog;
final private MessageType type;
final private MessageHandler handler;
final private boolean verbose;
Expand Down Expand Up @@ -34,8 +34,25 @@ public String getInternalLog() {
return internalLog;
}

public MessageType getType() {
return type;
}

public MessageHandler getHandler() {
return handler;
}

public boolean isVerbose() {
return verbose;
}

public long getTime() {
return time;
}

enum MessageHandler {
UX,
UX_SPINNER,
INTERNAL
}

Expand Down
16 changes: 13 additions & 3 deletions messages/EventKeyTemplates.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,15 @@ module.exports = {
"customEslintHeadsUp": "About to run Eslint with custom config in %s. Please make sure your current directory has all the required NPM dependencies.",
"customPmdHeadsUp": "About to run PMD with custom config in %s. Please make sure that any custom rule references have already been added to the plugin through scanner:rule:add command.",
"pmdRuleSkipped": "Omitting results for PMD rule \"%s\". Reason: %s.",
"unmatchedPathExtensionCpd": "Path extensions for the following files will not be processed by CPD: %s"
"unmatchedPathExtensionCpd": "Path extensions for the following files will not be processed by CPD: %s",
"sfgeInfoLog": "%s",
"sfgeMetaInfoCollected": "Loaded %s: [ %s ]",
"sfgeFinishedCompilingFiles": "Compiled %s files.",
"sfgeStartedBuildingGraph": "Building graph.",
"sfgeFinishedBuildingGraph": "Added all compilation units to graph.",
"sfgePathEntryPointsIdentified": "Identified %s path entry point(s).",
"sfgeViolationsInPathProgress": "Detected %s violation(s) from %s path(s) on %s/%s entry point(s).",
"sfgeCompletedPathAnalysis": "Overall, analyzed %s path(s) from %s entry point(s). Detected %s violation(s)."
},
"warning": {
"invalidCategorySkipped": "Cataloger skipped invalid PMD Category file '%s'.",
Expand All @@ -20,15 +28,17 @@ module.exports = {
"unexpectedPmdNodeType": "Encountered unexpected PMD node of type '%s'",
"multipleMethodTargetMatches": "Total of %s methods in file %s matched name #%s",
"noMethodTargetMatches": "No methods in file %s matched name #%s()",
"pmdConfigError": "PMD failed to evaluate rule '%s'. Message: %s"
"pmdConfigError": "PMD failed to evaluate rule '%s'. Message: %s",
"sfgeWarnLog": "%s"
},
"error": {
"internal": {
"unexpectedError": "INTERNAL ERROR: Unexpected error occurred while cataloging rules: %s. Please log an issue with us at github.com/forcedotcom/sfdx-scanner.",
"mainInvalidArgument": "INTERNAL ERROR: Invalid arguments passed to Main. Details: %s. Please log an issue with us at github.com/forcedotcom/sfdx-scanner.",
"jsonWriteFailed": "INTERNAL ERROR: Failed to write JSON to file: %s. Please log an issue with us at github.com/forcedotcom/sfdx-scanner.",
"classpathDoesNotExist": "INTERNAL ERROR: Path does not exist: %s. Please log an issue with us at github.com/forcedotcom/sfdx-scanner.",
"xmlMissingInClasspath": "INTERNAL ERROR: XML resource [%s] found in jar, but not in Classpath. Please log an issue with us at github.com/forcedotcom/sfdx-scanner."
"xmlMissingInClasspath": "INTERNAL ERROR: XML resource [%s] found in jar, but not in Classpath. Please log an issue with us at github.com/forcedotcom/sfdx-scanner.",
"sfgeErrorLog": "%s"
},
"external": {
"errorMessageAbove": "Please see error details displayed above.",
Expand Down
86 changes: 86 additions & 0 deletions sfge/src/main/java/com/salesforce/CliMessagerAppender.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
package com.salesforce;

import com.salesforce.config.SfgeConfigProvider;
import com.salesforce.messaging.CliMessager;
import com.salesforce.messaging.EventKey;
import java.io.Serializable;
import org.apache.logging.log4j.Level;
import org.apache.logging.log4j.core.Appender;
import org.apache.logging.log4j.core.Core;
import org.apache.logging.log4j.core.Filter;
import org.apache.logging.log4j.core.Layout;
import org.apache.logging.log4j.core.LogEvent;
import org.apache.logging.log4j.core.appender.AbstractAppender;
import org.apache.logging.log4j.core.config.plugins.Plugin;
import org.apache.logging.log4j.core.config.plugins.PluginAttribute;
import org.apache.logging.log4j.core.config.plugins.PluginElement;
import org.apache.logging.log4j.core.config.plugins.PluginFactory;
import org.apache.logging.log4j.core.layout.PatternLayout;

/**
* Custom log4j2 appender to send logs as realtime events through {@link CliMessager}. This helps
* streamline logs displayed to commandline users. Invoked from log4j.xml.
*/
@Plugin(
name = "CliMessagerAppender",
category = Core.CATEGORY_NAME,
elementType = Appender.ELEMENT_TYPE)
public class CliMessagerAppender extends AbstractAppender {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

New log appender that steps in to pass realtime messages when needed.


private final boolean shouldLogWarningsOnVerbose;

@PluginFactory
public static CliMessagerAppender createAppender(
@PluginAttribute("name") String name,
@PluginElement("Layout") Layout<? extends Serializable> layout,
@PluginElement("Filter") final Filter filter) {
if (name == null) {
// Assign default name to avoid complaining
name = "CliMessagerAppender";
}
if (layout == null) {
layout = PatternLayout.createDefaultLayout();
}
return new CliMessagerAppender(name, filter, layout, true);
}

protected CliMessagerAppender(
String name,
Filter filter,
Layout<? extends Serializable> layout,
final boolean ignoreExceptions) {
super(name, filter, layout, ignoreExceptions, null);
this.shouldLogWarningsOnVerbose = SfgeConfigProvider.get().shouldLogWarningsOnVerbose();
}

/**
* {@link CliMessagerAppender} decrements the log level while publishing to CLI. Warning is
* reduced to Info, Error is reduced to Warning, Fatal is reduced to Error.
*
* @param event that was published from code
*/
@Override
public void append(LogEvent event) {
Level level = event.getLevel();
if (Level.WARN.equals(level) && this.shouldLogWarningsOnVerbose) {
CliMessager.postMessage(
"SFGE Warning as Info", EventKey.INFO_GENERAL, getEventMessage(event));
} else if (Level.ERROR.equals(level)) {
CliMessager.postMessage(
"SFGE Error as Warning", EventKey.WARNING_GENERAL, getEventMessage(event));
} else if (Level.FATAL.equals(level)) {
CliMessager.postMessage(
"SFGE Fatal as Error", EventKey.ERROR_GENERAL, getEventMessage(event));
} else {
error(
String.format(
"Unable to log less than WARN level [{}]: {}",
event.getLevel(),
getEventMessage(event)));
}
}

private String getEventMessage(LogEvent event) {
return event.getMessage().getFormattedMessage();
}
}
7 changes: 7 additions & 0 deletions sfge/src/main/java/com/salesforce/Main.java
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
import com.salesforce.rules.RuleRunner;
import com.salesforce.rules.RuleUtil;
import com.salesforce.rules.Violation;
import com.salesforce.rules.ops.ProgressListenerProvider;
import java.util.Arrays;
import java.util.Collection;
import java.util.List;
Expand Down Expand Up @@ -146,6 +147,7 @@ private int execute(String... args) {
List<Violation> allViolations;
try {
allViolations = new RuleRunner(g).runRules(eap.getSelectedRules(), eap.getTargets());
ProgressListenerProvider.get().completedAnalysis();
} catch (SfgeRuntimeException ex) {
LOGGER.error("Error while running rules", ex);
System.err.println(formatError(ex));
Expand All @@ -172,6 +174,11 @@ private void collectMetaInfo(CliArgParser.ExecuteArgParser eap) {

for (MetaInfoCollector collector : allCollectors) {
collector.loadProjectFiles(eap.getProjectDirs());

// Let progress listener know about the meta information collected
ProgressListenerProvider.get()
.collectedMetaInfo(
collector.getMetaInfoTypeName(), collector.getMetaInfoCollected());
}
}

Expand Down
25 changes: 25 additions & 0 deletions sfge/src/main/java/com/salesforce/config/EnvUtil.java
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ public final class EnvUtil {
private static final String ENV_RULE_ENABLE_WARNING_VIOLATION =
"SFGE_RULE_ENABLE_WARNING_VIOLATION";
private static final String ENV_IGNORE_PARSE_ERRORS = "SFGE_IGNORE_PARSE_ERRORS";
private static final String ENV_LOG_WARNINGS_ON_VERBOSE = "SFGE_LOG_WARNINGS_ON_VERBOSE";
private static final String ENV_PROGRESS_INCREMENTS = "SFGE_PROGRESS_INCREMENTS";

// TODO: These should move to SfgeConfigImpl and this class should return Optionals
@VisibleForTesting
Expand All @@ -21,6 +23,8 @@ public final class EnvUtil {

@VisibleForTesting static final boolean DEFAULT_RULE_ENABLE_WARNING_VIOLATION = true;
@VisibleForTesting static final boolean DEFAULT_IGNORE_PARSE_ERRORS = false;
@VisibleForTesting static final boolean DEFAULT_LOG_WARNINGS_ON_VERBOSE = false;
@VisibleForTesting static final int DEFAULT_PROGRESS_INCREMENTS = 10;

/**
* Returns the value of the {@link #ENV_RULE_THREAD_COUNT} environment variable if set, else
Expand Down Expand Up @@ -61,6 +65,27 @@ static boolean shouldIgnoreParseErrors() {
return getBoolOrDefault(ENV_IGNORE_PARSE_ERRORS, DEFAULT_IGNORE_PARSE_ERRORS);
}

/**
* Indicates if SFGE should log internal warnings on --verbose
*
* @return value of {@link #ENV_LOG_WARNINGS_ON_VERBOSE} env variable. If it is not set, {@link
* #DEFAULT_LOG_WARNINGS_ON_VERBOSE}
*/
static boolean shouldLogWarningsOnVerbose() {
return getBoolOrDefault(ENV_LOG_WARNINGS_ON_VERBOSE, DEFAULT_LOG_WARNINGS_ON_VERBOSE);
}

/**
* Gets the level of increments at which path analysis progress update is provided. Applicable
* only with --verbose.
*
* @return value of {@link #ENV_PROGRESS_INCREMENTS} environment variable if set, else {@link
* #DEFAULT_PROGRESS_INCREMENTS}
*/
static int getProgressIncrements() {
return getIntOrDefault(ENV_PROGRESS_INCREMENTS, DEFAULT_PROGRESS_INCREMENTS);
}

private static int getIntOrDefault(String name, int defaultValue) {
String strVal = System.getProperty(name);
return strVal == null ? defaultValue : Integer.parseInt(strVal);
Expand Down
12 changes: 12 additions & 0 deletions sfge/src/main/java/com/salesforce/config/SfgeConfig.java
Original file line number Diff line number Diff line change
Expand Up @@ -16,4 +16,16 @@ public interface SfgeConfig {

/** Indicates if a Jorje parse error causes the entire process to stop. */
boolean shouldIgnoreParseErrors();

Copy link
Contributor Author

Choose a reason for hiding this comment

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

New config values to control verbose behavior.
LogWarningsOnVerbose would make the output noisier by including all the warnings from SFGE as info events.
ProgressIncrementsOnVerbose controls how often we post updates on number of paths analyzed.

Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need these config values anymore? Would removing them simplify the code?

/**
* Indicates if Warn level logs to log4j should be forwarded to CLI as well when verbose is
* enabled
*/
boolean shouldLogWarningsOnVerbose();

/**
* Should be used to set the level of increments at which path analysis progress update is
* provided
*/
int getProgressIncrements();
}
Loading