Skip to content

Commit

Permalink
Improves rethrow exceptions from throw e; to just throw; to avoid los…
Browse files Browse the repository at this point in the history
…s of stack data info and removes unused lines of code for cleanliness identified by LGTM/Semmle. Fixes loss of confidence data in some cases from serialization where not specified. Fixes one hardcoded output filename for tagtest and prevents msg for console winding up in log. (#185)
  • Loading branch information
guyacosta committed Mar 28, 2020
1 parent eecbc67 commit 9283917
Show file tree
Hide file tree
Showing 12 changed files with 138 additions and 97 deletions.
20 changes: 10 additions & 10 deletions AppInspector/Commands/AnalyzeCommand.cs
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ public AnalyzeCommand(AnalyzeCommandOptions opt)
Utils.Logger = null;
WriteOnce.Log = null;
}
throw e;
throw;
}

_uniqueTagsControl = new HashSet<string>();
Expand Down Expand Up @@ -326,7 +326,7 @@ public override string GetResult()
public override int Run()
{
WriteOnce.SafeLog("AnalyzeCommand::Run", LogLevel.Trace);
WriteOnce.Operation(ErrMsg.FormatString(ErrMsg.ID.CMD_RUNNING, "Analyze"));
WriteOnce.Operation(ErrMsg.FormatString(ErrMsg.ID.CMD_RUNNING, "analyze"));

try
{
Expand Down Expand Up @@ -369,7 +369,7 @@ public override int Run()
WriteOnce.Error(ErrMsg.GetString(ErrMsg.ID.ANALYZE_NOPATTERNS));
else
{
WriteOnce.Operation(ErrMsg.FormatString(ErrMsg.ID.CMD_COMPLETED, "Analyze"));
WriteOnce.Operation(ErrMsg.FormatString(ErrMsg.ID.CMD_COMPLETED, "analyze"));
if (!_arg_autoBrowserOpen)
WriteOnce.Any(ErrMsg.FormatString(ErrMsg.ID.ANALYZE_OUTPUT_FILE, "output.html"), true, ConsoleColor.Gray, WriteOnce.ConsoleVerbosity.Low);
}
Expand All @@ -381,7 +381,7 @@ public override int Run()
if (Utils.CLIExecutionContext)
return (int)ExitCode.CriticalError;
else
throw e;
throw;
}
finally
{
Expand Down Expand Up @@ -609,7 +609,7 @@ private string ExtractExcerpt(string text, int startLineNumber, int length = 10)
* space, so we'll find the smallest number of spaces at the beginning of
* each line and use that.
*/
var n = (int)Math.Floor(Math.Log10(excerptEndLine) + 1);

var minSpaces = -1;
for (var i = excerptStartLine; i <= excerptEndLine; i++)
{
Expand Down Expand Up @@ -650,7 +650,7 @@ public void FlushAll()
if (_arg_consoleVerbosityLevel.ToLower() != "none")
{
if (!String.IsNullOrEmpty(_arg_outputFile) && Utils.CLIExecutionContext)
WriteOnce.Any(ErrMsg.FormatString(ErrMsg.ID.ANALYZE_OUTPUT_FILE, _arg_outputFile), true, ConsoleColor.Gray, WriteOnce.ConsoleVerbosity.Medium);
WriteOnce.Info(ErrMsg.FormatString(ErrMsg.ID.ANALYZE_OUTPUT_FILE, _arg_outputFile), true, WriteOnce.ConsoleVerbosity.Medium, false);
else
WriteOnce.NewLine();
}
Expand Down Expand Up @@ -710,11 +710,11 @@ void UnZipAndProcess(string filename, ArchiveFileType archiveFileType)
}

}
catch (Exception e)
catch (Exception)
{
string errmsg = ErrMsg.FormatString(ErrMsg.ID.ANALYZE_COMPRESSED_ERROR, filename);
WriteOnce.Error(errmsg);
throw e;
throw;
}

}
Expand Down Expand Up @@ -763,10 +763,10 @@ bool FileChecksPassed(string filePath, ref LanguageInfo languageInfo, long fileL
return false;
}
}
catch (Exception e)
catch (Exception)
{
WriteOnce.Error(ErrMsg.FormatString(ErrMsg.ID.CMD_INVALID_FILE_OR_DIR, filePath));
throw e;
throw;
}

return true;
Expand Down
55 changes: 35 additions & 20 deletions AppInspector/Commands/ExportTagsCommand.cs
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ public ExportTagsCommand(ExportTagsCommandOptions opt)
Utils.Logger = null;
WriteOnce.Log = null;
}
throw e;
throw;
}
}

Expand Down Expand Up @@ -160,32 +160,47 @@ public override int Run()

SortedDictionary<string, string> uniqueTags = new SortedDictionary<string, string>();

foreach (Rule r in _rules)
try
{
//builds a list of unique tags
foreach (string t in r.Tags)

foreach (Rule r in _rules)
{
if (uniqueTags.ContainsKey(t))
continue;
else
uniqueTags.Add(t, t);
//builds a list of unique tags
foreach (string t in r.Tags)
{
if (uniqueTags.ContainsKey(t))
continue;
else
uniqueTags.Add(t, t);
}
}
}

//separate loop so results are sorted (Sorted type)
foreach (string s in uniqueTags.Values)
WriteOnce.Result(s, true);

WriteOnce.Operation(ErrMsg.FormatString(ErrMsg.ID.CMD_COMPLETED, "Exporttags"));
if (!String.IsNullOrEmpty(_arg_outputFile) && Utils.CLIExecutionContext)
WriteOnce.Any(ErrMsg.FormatString(ErrMsg.ID.ANALYZE_OUTPUT_FILE, _arg_outputFile), true, ConsoleColor.Gray, WriteOnce.ConsoleVerbosity.Low);
//separate loop so results are sorted (Sorted type)
foreach (string s in uniqueTags.Values)
WriteOnce.Result(s, true);

WriteOnce.FlushAll();
WriteOnce.Operation(ErrMsg.FormatString(ErrMsg.ID.CMD_COMPLETED, "Exporttags"));
if (!String.IsNullOrEmpty(_arg_outputFile) && Utils.CLIExecutionContext)
WriteOnce.Info(ErrMsg.FormatString(ErrMsg.ID.ANALYZE_OUTPUT_FILE, _arg_outputFile), true, WriteOnce.ConsoleVerbosity.Low, false);

if (_arg_close_log_on_exit)
WriteOnce.FlushAll();
}
catch (Exception e)
{
Utils.Logger = null;
WriteOnce.Log = null;
WriteOnce.Error(e.Message);
//exit normaly for CLI callers and throw for DLL callers
if (Utils.CLIExecutionContext)
return (int)ExitCode.CriticalError;
else
throw;
}
finally
{
if (_arg_close_log_on_exit)
{
Utils.Logger = null;
WriteOnce.Log = null;
}
}

return (int)ExitCode.Success;
Expand Down
8 changes: 4 additions & 4 deletions AppInspector/Commands/PackRulesCommand.cs
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ public PackRulesCommand(PackRulesCommandOptions opt)
Utils.Logger = null;
WriteOnce.Log = null;
}
throw e;
throw;
}
}

Expand Down Expand Up @@ -134,7 +134,7 @@ public override string GetResult()
public override int Run()
{
WriteOnce.SafeLog("PackRules::Run", LogLevel.Trace);
WriteOnce.Operation(ErrMsg.FormatString(ErrMsg.ID.CMD_RUNNING, "PackRules"));
WriteOnce.Operation(ErrMsg.FormatString(ErrMsg.ID.CMD_RUNNING, "packrules"));

try
{
Expand All @@ -154,7 +154,7 @@ public override int Run()
fs.Close();
}

WriteOnce.Operation(ErrMsg.FormatString(ErrMsg.ID.CMD_COMPLETED, "PackRules"));
WriteOnce.Operation(ErrMsg.FormatString(ErrMsg.ID.CMD_COMPLETED, "packrules"));
WriteOnce.Any(ErrMsg.FormatString(ErrMsg.ID.ANALYZE_OUTPUT_FILE, _arg_outputfile), true, ConsoleColor.Gray, WriteOnce.ConsoleVerbosity.Medium);
WriteOnce.FlushAll();
}
Expand All @@ -165,7 +165,7 @@ public override int Run()
if (Utils.CLIExecutionContext)
return (int)ExitCode.CriticalError;
else
throw e;
throw;
}
finally
{
Expand Down
10 changes: 5 additions & 5 deletions AppInspector/Commands/TagDiffCommand.cs
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ public TagDiffCommand(TagDiffCommandOptions opt)
Utils.Logger = null;
WriteOnce.Log = null;
}
throw e;
throw;
}
}

Expand Down Expand Up @@ -161,7 +161,7 @@ public override string GetResult()
public override int Run()
{
WriteOnce.SafeLog("TagDiffCommand::Run", LogLevel.Trace);
WriteOnce.Operation(ErrMsg.FormatString(ErrMsg.ID.CMD_RUNNING, "Tagdiff"));
WriteOnce.Operation(ErrMsg.FormatString(ErrMsg.ID.CMD_RUNNING, "tagdiff"));

ExitCode exitCode = ExitCode.CriticalError;
//save to quiet analyze cmd and restore
Expand Down Expand Up @@ -264,9 +264,9 @@ public override int Run()

WriteOnce.General(ErrMsg.FormatString(ErrMsg.ID.TAGDIFF_RESULTS_DIFFER), false);
WriteOnce.Result(resultsDiffer.ToString());
WriteOnce.Operation(ErrMsg.FormatString(ErrMsg.ID.CMD_COMPLETED, "Tagdiff"));
WriteOnce.Operation(ErrMsg.FormatString(ErrMsg.ID.CMD_COMPLETED, "tagdiff"));
if (!String.IsNullOrEmpty(_arg_outputFile) && Utils.CLIExecutionContext)
WriteOnce.Any(ErrMsg.FormatString(ErrMsg.ID.ANALYZE_OUTPUT_FILE, _arg_outputFile), true, ConsoleColor.Gray, WriteOnce.ConsoleVerbosity.Low);
WriteOnce.Info(ErrMsg.FormatString(ErrMsg.ID.ANALYZE_OUTPUT_FILE, _arg_outputFile), true, WriteOnce.ConsoleVerbosity.Low, false);

WriteOnce.FlushAll();
}
Expand Down Expand Up @@ -294,7 +294,7 @@ public override int Run()
if (Utils.CLIExecutionContext)
return (int)ExitCode.CriticalError;
else
throw e;
throw;
}
finally
{
Expand Down
9 changes: 4 additions & 5 deletions AppInspector/Commands/TagTestCommand.cs
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ public TagTestCommand(TagTestCommandOptions opt)
Utils.Logger = null;
WriteOnce.Log = null;
}
throw e;
throw;
}
}

Expand Down Expand Up @@ -221,7 +221,6 @@ public override int Run()
analyzeCmdResult = (AnalyzeCommand.ExitCode)cmd1.Run();

//must be done here to avoid losing our handle from analyze command overwriting WriteOnce.Writer
_arg_outputFile ??= "output.json";
ConfigureFileOutput();

//restore
Expand All @@ -239,7 +238,7 @@ public override int Run()
else
WriteOnce.Any(ErrMsg.GetString(ErrMsg.ID.TAGTEST_RESULTS_SUCCESS), true, ConsoleColor.Green, WriteOnce.ConsoleVerbosity.Low);

WriteOnce.Operation(ErrMsg.FormatString(ErrMsg.ID.CMD_COMPLETED, "Tagtest"));
WriteOnce.Operation(ErrMsg.FormatString(ErrMsg.ID.CMD_COMPLETED, "tagtest"));

exitCode = _arg_tagTestType == TagTestType.RulesPresent ? ExitCode.SomeTagsFailed : ExitCode.TagsTestSuccess;
}
Expand Down Expand Up @@ -287,7 +286,7 @@ public override int Run()

WriteOnce.Operation(ErrMsg.FormatString(ErrMsg.ID.CMD_COMPLETED, "Tagtest"));
if (!String.IsNullOrEmpty(_arg_outputFile) && Utils.CLIExecutionContext)
WriteOnce.Any(ErrMsg.FormatString(ErrMsg.ID.ANALYZE_OUTPUT_FILE, _arg_outputFile), true, ConsoleColor.Gray, WriteOnce.ConsoleVerbosity.Low);
WriteOnce.Info(ErrMsg.FormatString(ErrMsg.ID.ANALYZE_OUTPUT_FILE, _arg_outputFile), true, WriteOnce.ConsoleVerbosity.Low, false);

WriteOnce.FlushAll();
}
Expand All @@ -304,7 +303,7 @@ public override int Run()
if (Utils.CLIExecutionContext)
return (int)ExitCode.CriticalError;
else
throw e;
throw;
}
finally
{
Expand Down
10 changes: 5 additions & 5 deletions AppInspector/Commands/VerifyRulesCommand.cs
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ public VerifyRulesCommand(VerifyRulesCommandOptions opt)
Utils.Logger = null;
WriteOnce.Log = null;
}
throw e;
throw;
}
}

Expand Down Expand Up @@ -140,7 +140,7 @@ public override string GetResult()
public override int Run()
{
WriteOnce.SafeLog("VerifyRulesCommand::Run", LogLevel.Trace);
WriteOnce.Operation(ErrMsg.FormatString(ErrMsg.ID.CMD_RUNNING, "Verify Rules"));
WriteOnce.Operation(ErrMsg.FormatString(ErrMsg.ID.CMD_RUNNING, "verifyrules"));

ExitCode exitCode = ExitCode.CriticalError;

Expand All @@ -159,7 +159,7 @@ public override int Run()
}

WriteOnce.Any(ErrMsg.GetString(ErrMsg.ID.VERIFY_RULES_RESULTS_SUCCESS), true, ConsoleColor.Green, WriteOnce.ConsoleVerbosity.Low);
WriteOnce.Operation(ErrMsg.FormatString(ErrMsg.ID.CMD_COMPLETED, "Verify Rules"));
WriteOnce.Operation(ErrMsg.FormatString(ErrMsg.ID.CMD_COMPLETED, "verifyrules"));
if (!String.IsNullOrEmpty(_arg_outputFile) && Utils.CLIExecutionContext)
WriteOnce.Any(ErrMsg.FormatString(ErrMsg.ID.ANALYZE_OUTPUT_FILE, _arg_outputFile), true, ConsoleColor.Gray, WriteOnce.ConsoleVerbosity.Low);
WriteOnce.FlushAll();
Expand All @@ -169,9 +169,9 @@ public override int Run()
WriteOnce.Error(e.Message);
//exit normaly for CLI callers and throw for DLL callers
if (Utils.CLIExecutionContext)
return (int)ExitCode.CriticalError;//no way to distinguish from critical fail
return (int)ExitCode.CriticalError;
else
throw e;
throw;
}
finally
{
Expand Down
26 changes: 8 additions & 18 deletions AppInspector/RulesVerifier.cs
Original file line number Diff line number Diff line change
Expand Up @@ -24,40 +24,33 @@ public RulesVerifier(string rulePath)
}


public bool Verify()
public void Verify()
{
bool isCompiled = true;

if (Directory.Exists(_rulesPath))
isCompiled = LoadDirectory(_rulesPath);
LoadDirectory(_rulesPath);
else if (File.Exists(_rulesPath))
isCompiled = LoadFile(_rulesPath);
LoadFile(_rulesPath);
else
{
throw new Exception(ErrMsg.FormatString(ErrMsg.ID.CMD_INVALID_RULE_PATH, _rulesPath));
}

return isCompiled;
}



private bool LoadDirectory(string path)
private void LoadDirectory(string path)
{
bool result = true;
foreach (string filename in Directory.EnumerateFileSystemEntries(path, "*.json", SearchOption.AllDirectories))
{
if (!LoadFile(filename))
result = false;
LoadFile(filename);
}

return result;
}

private bool LoadFile(string file)
private void LoadFile(string file)
{
RuleSet rules = new RuleSet();
bool noProblem = true;

try
{
rules.AddFile(file, null);
Expand All @@ -69,10 +62,7 @@ private bool LoadFile(string file)
throw new Exception(ErrMsg.FormatString(ErrMsg.ID.VERIFY_RULE_FAILED, file));
}

if (noProblem)
_rules.AddRange(rules.AsEnumerable());

return noProblem;
_rules.AddRange(rules.AsEnumerable());
}


Expand Down
2 changes: 0 additions & 2 deletions AppInspector/Utils.cs
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,6 @@ static public RuleSet GetDefaultRuleSet(Logger logger = null)
{
RuleSet ruleSet = new RuleSet(logger);
Assembly assembly = Assembly.GetExecutingAssembly();
string[] resourceName = assembly.GetManifestResourceNames();
string filePath = "Microsoft.ApplicationInspector.Commands.defaultRulesPkd.json";
Stream resource = assembly.GetManifestResourceStream(filePath);
using (StreamReader file = new StreamReader(resource))
Expand Down Expand Up @@ -296,7 +295,6 @@ public static Logger SetupLogging(AllCommandOptions opts)
LogManager.Configuration = config;
Logger = LogManager.GetLogger("CST.ApplicationInspector");
return Logger;

}

}
Expand Down

0 comments on commit 9283917

Please sign in to comment.