microsoft / sarif-pattern-matcher

Quality domain agnostic regular expression pattern matcher that persists results to SARIF
MIT License
39 stars 17 forks source link

Exposing MatchExpressions in SearchSkimmer #759

Closed suvamM closed 1 year ago

suvamM commented 1 year ago

Changes

Various recent activities have the following pattern: we consume the JSON plug-in, and parse the regular expressions. The typical way to do this is to deserialize the JSON into a SearchDefinitions object, and then access the MatchExpressions list. However, this logic already exists in the CreateSkimmersFromDefinitions method, which creates a set of Skimmer objects. The problem: the match expressions were private.

This PR exposes the MatchExpression set as public in the SearchSkimmer class.

Sample usage:

ISet<Skimmer<AnalyzeContext>> skimmers = CreateSkimmersFromDefinitions(pluginsPath);
foreach (Skimmer<AnalyzeContext> skimmer in skimmers)
{
  var ss = (SearchSkimmer)skimmer;
  var regex = ss.MatchExpression.ContentsRegex;
}
LingZhou-gh commented 1 year ago

The match expression was changed from Private to Public in the PR, then where and how this public property will be actually used for? That is what is the need to make it public?

yongyan-gh commented 1 year ago

Should we add a release record for it?

suvamM commented 1 year ago

The match expression was changed from Private to Public in the PR, then where and how this public property will be actually used for? That is what is the need to make it public?

@LingZhou-gh I tried to describe this in the PR description: we have several scenarios where we are parsing the regular expressions related to the rules. Concrete examples include the synthetic dataset generation, and also sniff regex generation. Without making the field public, we would have to duplicate the parsing logic.

suvamM commented 1 year ago

Should we add a release record for it?

@yongyan-gh I thought about it, but I don't really think this change is so large as to warrant an entry in the Release Notes.