Merge pull request #23 from ethanis/content-filters

Add contentFilters to match comments based on commit diffs

.github/nitpicks.yml

@@ -1,15 +1,22 @@
 - markdown: |
     ## Rockstar alert
     Thanks for this contribution, we _really_ appreciate your help :heart:!
+
+- markdown: |
+    Are you _sure_ you want to be console logging or debuggering here?? :trollface:
   pathFilter:
-    - '*'
+    - 'src/**'
+  contentFilter:
+    - '(\+\s*console.log\(.*\))'
+    - '(\+\s*debugger)'
+
 - markdown: |
     Please proofread any changes to documentation.
   pathFilter:
     - '**/*.md'
+
 - markdown: |
-    ## Uh oh
-    Don't check in binaries...we use GitHub package registry for that!
+    Uhh... don't check in binaries here - we use GitHub package registry for that!
   blocking: true
   pathFilter:
     - '+**/*.dll'

README.md

@@ -10,14 +10,21 @@ Add a new YAML file that specifies the comments to be evaluated during PRs. We r
 - markdown: |
     ## Rockstar alert
     Thanks for this contribution, we _really_ appreciate your help :heart:!
-  pathFilter:
-    - '*'
+
 - markdown: |
     ## Uh oh
     Don't check in binaries...we use GitHub Packages for those!
   blocking: true
   pathFilter:
     - '+**/*.dll'
+
+- markdown: |
+    Are you _sure_ you want to be console logging or debuggering here?? :trollface:
+  pathFilter:
+    - 'src/**'
+  contentFilter:
+    - '(\+\s*console.log\(.*\))'
+    - '(\+\s*debugger)'
 ```
 
 Add the Nitpicker action to your workflow and allow access to the `secrets.GITHUB_TOKEN` variable.
@@ -69,6 +76,8 @@ In order for this PR to be completed, the author of the PR should address any co
 
 Path filters define which comments to add to a PR based on the set of files changed. These can either be explicit file paths or a file path pattern. If you want to add multiple path filters to a comment, they must be separated with a `,` or `;`. Comments will only be added to a PR if it changes files that match the path filter.
 
+If no value for `pathFilter` is specified, `*`, is used. This effectively means any file path will match the comment.
+
 Paths prefixed with `!` are excluded. For example, `!tests/**` will exclude files changed within the `tests/` directory.
 
 Paths prefixed with `+` indicate only _new_ files will match. For example, `+depot/*.dll` will match when a file with the file extension `.dll` is added to the `depot/` directory but would not match when an existing `.dll` is edited or deleted in the `depot/` directory.
@@ -77,6 +86,23 @@ Paths prefixed with `-` indicate only _removed_ files will match. For example, `
 
 Paths prefixed with `~` indicate only _edited_ files will match. For example, `~sources/*.nupkg` will match when an existing file with the file extension `.nupkg` is edited in the `sources/` directory, but would not match when a `.nugpkg` file is added or deleted in the `sources/` directory.
 
+## Content filters
+
+Content filters define which comments to add based on the `patch` of the files changed.
+
+A comment's `contentFilter` parameter accepts an array of regular expressions that are used to match comments based on the changes to a file in a PR. These can be used in combination with path filters to create complex comment rules. For example, this rule will match any typescript file in the `src` directory, except `src/models/debug.ts`, that add a `console.log` or `debugger` statement.
+
+```yaml
+- markdown: |
+    Did you _really_ mean to commit this debug statement??
+  pathFilter:
+    - 'src/**/*.ts'
+    - '!src/models/debug.ts'
+  contentFilter:
+    - '(\+\s*console.log\(.*\))'
+    - '(\+\s*debugger)'
+```
+
 ## Development
 
 Install the dependencies

__tests__/configReader.test.ts

@@ -3,16 +3,34 @@ import { getConfiguredComments } from '../src/services';
 import { Constants } from '../src/constants';
 
 test('read valid config file', () => {
-  const configFile = './data/valid-nitpicks.yml';
+  const configFile = './data/valid_nitpicks.yml';
   const inputPath = path.join(__dirname, configFile);
 
   const comments = getConfiguredComments(inputPath);
 
-  expect(comments.length).toEqual(3);
+  expect(comments.length).toEqual(5);
+});
+
+test("pathFilter defaults to '*'", () => {
+  const configFile = './data/default_path_filter.yml';
+  const inputPath = path.join(__dirname, configFile);
+
+  const comments = getConfiguredComments(inputPath);
+
+  expect(comments.every(c => c.pathFilter.every(p => p === '*'))).toBeTruthy();
+});
+
+test('contentFilter defaults to undefined', () => {
+  const configFile = './data/default_content_filter.yml';
+  const inputPath = path.join(__dirname, configFile);
+
+  const comments = getConfiguredComments(inputPath);
+
+  expect(comments.every(c => !c.contentFilter)).toBeTruthy();
 });
 
 test('read invalid config file', () => {
-  const configFile = './data/invalid-nitpicks.yml';
+  const configFile = './data/invalid_nitpicks.yml';
   const inputPath = path.join(__dirname, configFile);
 
   const comments = getConfiguredComments(inputPath);
@@ -21,7 +39,7 @@ test('read invalid config file', () => {
 });
 
 test('blocking comment text', () => {
-  const configFile = './data/blocking-nitpicks.yml';
+  const configFile = './data/blocking_nitpicks.yml';
   const inputPath = path.join(__dirname, configFile);
 
   const comments = getConfiguredComments(inputPath);
@@ -33,3 +51,12 @@ test('blocking comment text', () => {
     }
   }
 });
+
+test('file doesnt exist', () => {
+  const configFile = './data/foo_bar.yml';
+  const inputPath = path.join(__dirname, configFile);
+
+  expect(() => getConfiguredComments(inputPath)).toThrowError(
+    'Nitpicks file does not exist: '
+  );
+});

__tests__/data/blocking-nitpicks.yml → __tests__/data/blocking_nitpicks.yml

@@ -1,5 +1,5 @@
 - markdown: |
-    This is a blcoking comment
+    This is a blocking comment
   blocking: true
   pathFilter:
     - '**/*.dll'

__tests__/data/default_content_filter.yml

@@ -0,0 +1,9 @@
+- markdown: |
+    ## Notice
+    Thanks for this contribution. We _really_ appreciate your help!
+
+- markdown: |
+    ## Blocking issue
+  blocking: true
+  pathFilter:
+    - '**/*.dll'

__tests__/data/default_path_filter.yml

@@ -0,0 +1,8 @@
+- markdown: |
+    ## Notice
+    Thanks for this contribution. We _really_ appreciate your help!
+
+- markdown: |
+    Properly escaped regexp
+  contentFilter:
+    - '(\+\s*console.log\(.*\))'

__tests__/data/valid_nitpicks.yml

@@ -0,0 +1,30 @@
+# Test that pathFilter alone works
+- markdown: |
+    ## Notice
+    Thanks for this contribution. We _really_ appreciate your help!
+  pathFilter:
+    - '**'
+# blocking is a valid option
+- markdown: |
+    ## Blocking issue
+    We no longer check in binaries to this repo - our new convention is to use NuGet packages.
+
+    Read more about this change [here](https://www.nuget.org/).
+  blocking: true
+  pathFilter:
+    - '**/*.dll'
+# Path filter should default to *
+- markdown: |
+    Apply to everything
+# Test that contentFilter is a valid option
+- markdown: |
+    Don't add email addresses!
+  contentFilter:
+    - '[\w-]+@([\w-]+\.)+[\w-]+'
+# Kitchen sink
+- markdown: |
+    Oops - looks like you added a `puts` statement
+  pathFilter:
+    - app/**/*.rb
+  contentFilter:
+    - '(puts\W+)'