Merge pull request #24 from pjozsef/master

Add option to ignore single line comments

Author: designfrontier

.gitignore

@@ -1 +1,2 @@
-node_modules/
+node_modules/
+.idea

README.md

@@ -70,17 +70,21 @@ look like this:
   "exclude": {
     "users": ["[email protected]", "[email protected]", "[email protected]"],
     "files": ["readme.md", "history.md", "AUTHORS"],
-    "directories": ["bin"]
+    "directories": ["bin"],
+    "comments": { ".rb": "#", ".js": "//" }
   },
   "threshold": 20
 }
 ```
 
+With `exclude.comments`, you can exclude lines starting with specific characters 
+for the given file extensions. 
+
 ### command line argument
 
 You may also pass the location of your config file ar runtime with the `--config`
 option. This allows you to store your config in whatever file you would like. It
 still needs to conform to the structure laid out above though.
 
 As an example: `pythia --config pythia.json` where `pythia.json` is at the root
-of your project.
+of your project.

fixtures/.empty-config

@@ -0,0 +1,2 @@
+{
+}

fixtures/.exclude-comments-config

@@ -0,0 +1,8 @@
+{
+  "exclude": {
+    "comments": {
+        "rb": "#",
+        "js": "//"
+    }
+  }
+}

fixtures/.exclude-directories-config

@@ -0,0 +1,8 @@
+{
+  "exclude": {
+    "directories": [
+      "dir-one",
+      "dir-two"
+    ]
+  }
+}

fixtures/.exclude-files-config

@@ -0,0 +1,8 @@
+{
+  "exclude": {
+    "files": [
+      "file-one",
+      "file-two"
+    ]
+  }
+}

fixtures/.exclude-users-config

@@ -0,0 +1,8 @@
+{
+  "exclude": {
+    "users": [
+      "[email protected]",
+      "[email protected]"
+    ]
+  }
+}

fixtures/.malformed-config

@@ -0,0 +1,3 @@
+{
+  malformed: json
+}

fixtures/.pythia-config

@@ -0,0 +1,21 @@
+{
+  "threshold": 43,
+  "exclude": {
+    "users": [
+      "[email protected]",
+      "[email protected]"
+    ],
+    "directories": [
+      "dir-one",
+      "dir-two"
+    ],
+    "files": [
+      "file-one",
+      "file-two"
+    ],
+    "comments": {
+        "rb": "#",
+        "js": "//"
+    }
+  }
+}

fixtures/.threshold-config

@@ -0,0 +1,3 @@
+{
+  "threshold": 42
+}

fixtures/blame-comment-single-line.txt

@@ -0,0 +1,2 @@
+9a93ec03 (<[email protected]> 2018-04-03 22:10:49 -0600 1) # ruby comment here
+9a93ec03 (<[email protected]> 2018-04-03 22:10:49 -0600 1) puts 'hello'

fixtures/blame-multiple-comment.txt

@@ -0,0 +1,6 @@
+9a93ec03 (<[email protected]> 2018-04-03 22:10:49 -0600 1)  // js comment here 1
+9a93ec03 (<[email protected]> 2018-04-03 22:10:49 -0600 2)  console.log("a");
+9a93ec03 (<[email protected]> 2018-04-03 22:10:49 -0600 3)  // js comment here 2
+9a93ec03 (<[email protected]> 2018-04-03 22:10:49 -0600 4)  console.log("b");
+8a83ec04 (<[email protected]>   2018-04-03 23:10:49 -0600 5)  console.log("c");
+8a83ec04 (<[email protected]>   2018-04-03 23:10:49 -0600 6)  // js comment here 3

fixtures/blame.txt

@@ -0,0 +1,4 @@
+9a93ec03 (<[email protected]> 2018-04-03 22:10:49 -0600 1) line 1
+9a93ec03 (<[email protected]> 2018-04-03 22:10:49 -0600 2) line 2
+8a83ec04 (<[email protected]>   2018-04-03 23:10:49 -0600 3) line 3
+9a93ec03 (<[email protected]> 2018-04-03 22:10:49 -0600 4) line 4

fixtures/code-comment-single-line.js

@@ -0,0 +1,6 @@
+ // js comment here 1
+ console.log("a");
+ // js comment here 2
+ console.log("b");
+ console.log("c");
+ // js comment here 3

fixtures/code-comment-single-line.rb

@@ -0,0 +1,2 @@
+ # ruby comment here
+ puts 'hello'

modules/check-ownership.js

@@ -1,7 +1,7 @@
 const cp = require('child_process')
       path = require('path');
 
-module.exports = (details, publish) => {
+module.exports.checkOwnership = (details, publish) => {
   const { size, ownedLines, author, filePath, currentAuthor, threshold } = details;
   const ownership = (ownedLines / size) * 100;
 

modules/process-file.js

@@ -1,32 +1,39 @@
 const fs = require('fs'),
-      cp = require('child_process'),
-      split = require('split'),
-      getEmail = require('./get-email'),
-      checkOwnership = require('./check-ownership'),
-      getFileLength = (file, cb) => {
-        let lines = 0;
-        
-        if (!file) {
-          cb();
-          return;
+  cp = require('child_process'),
+  path = require('path'),
+  split = require('split'),
+  getEmail = require('./get-email'),
+  utils = require('./check-ownership'),
+  isCommentLine = (line, commentChar) => commentChar && RegExp(`^\\s*${commentChar}.*`, 'i').test(line),
+  getFileLength = (file, config, cb) => {
+    const commentChar = config.exclude.comments[path.extname(file)];
+    const commentIndices = [];
+    let currentLine = 0;
+
+    if (!file) {
+      cb();
+      return;
+    }
+
+    fs.createReadStream(file)
+      .pipe(split(null, null, { trailing: false }))
+      .on('data', (chunk) => {
+        if (isCommentLine(chunk, commentChar)) {
+          commentIndices.push(currentLine);
         }
-        
-        fs.createReadStream(file)
-          .pipe(split())
-          .on('data', (chunk) => {
-            lines++;
-          })
-          .on('end', () => {
-            cb(lines);
-          });
-      };
+        currentLine++;
+      })
+      .on('end', () => {
+        cb(currentLine - commentIndices.length, commentIndices);
+      });
+  };
 
 module.exports = ({file, excludeUsers, publish, currentAuthor, config}) => {
-  getFileLength(file, (lines) => {
+  getFileLength(file, config, (lines, commentIndices) => {
     cp.exec(`git blame -w --show-email HEAD~1 -- ${file}`, (error, stdout, stderr) => {
-      const emails = stdout.split('\n').reduce((accum, line) => {
+      const emails = stdout.split('\n').reduce((accum, line, lineIndex) => {
         const email = getEmail(line);
-        if (!email || excludeUsers.includes(email)) {
+        if (!email || excludeUsers.includes(email) || commentIndices.includes(lineIndex)) {
           return accum;
         }
 
@@ -37,14 +44,15 @@ module.exports = ({file, excludeUsers, publish, currentAuthor, config}) => {
         }
 
         return accum;
+
       }, {});
 
       Object.keys(emails).forEach((email) => {
         // { size, ownedLines, author, file_path }
-        checkOwnership({
-          size: lines, 
+        utils.checkOwnership({
+          size: lines,
           ownedLines: emails[email],
-          author: email, 
+          author: email,
           filePath: file,
           currentAuthor,
           threshold: config.threshold

modules/process-file.test.js

@@ -0,0 +1,120 @@
+const test = require('ava');
+const sinon = require('sinon');
+const processFile = require('./process-file');
+const fs = require('fs');
+const cp = require('child_process');
+const utils = require('./check-ownership');
+
+const sandbox = sinon.createSandbox();
+
+test.beforeEach(t => sandbox.restore());
+
+test.cb('calls checkOwnership with the appropriate arguments', t => {
+  t.plan(4);
+
+  const blameResult = fs.readFileSync('fixtures/blame.txt', 'utf8');
+
+  const checkOwnershipStub = sandbox.stub(utils, 'checkOwnership');
+  const file = 'fixtures/blame.txt';
+  sandbox.stub(cp, 'exec').withArgs(`git blame -w --show-email HEAD~1 -- ${file}`).yields(undefined, blameResult, undefined);
+  const excludeUsers = [];
+  const publish = false;
+  const currentAuthor = '[email protected]';
+  const config = { threshold: 42, exclude: { comments: {} } };
+
+  checkOwnershipStub.onCall(0).callsFake((params, givenPublish) => {
+    t.deepEqual(params, {
+      author: '[email protected]',
+      currentAuthor: currentAuthor,
+      filePath: file,
+      ownedLines: 3,
+      size: 4,
+      threshold: config.threshold,
+    });
+    t.is(givenPublish, publish);
+  });
+  checkOwnershipStub.onCall(1).callsFake((params, givenPublish) => {
+    t.deepEqual(params, {
+      author: '[email protected]',
+      currentAuthor: currentAuthor,
+      filePath: file,
+      ownedLines: 1,
+      size: 4,
+      threshold: config.threshold,
+    });
+    t.is(givenPublish, publish);
+    t.end();
+  });
+
+  processFile({ file, excludeUsers, publish, currentAuthor, config });
+});
+
+test.cb('calls checkOwnership without commented lines', t => {
+
+  const blameResult = fs.readFileSync('fixtures/blame-comment-single-line.txt', 'utf8');
+
+  const checkOwnershipStub = sandbox.stub(utils, 'checkOwnership');
+  const file = 'fixtures/code-comment-single-line.rb';
+  sandbox.stub(cp, 'exec').withArgs(`git blame -w --show-email HEAD~1 -- ${file}`).yields(undefined, blameResult, undefined);
+  const excludeUsers = [];
+  const publish = false;
+  const currentAuthor = '[email protected]';
+  const config = {
+    threshold: 42,
+    exclude: {
+      comments: { '.rb': '#' }
+    }
+  };
+
+  checkOwnershipStub.onCall(0).callsFake((params, givenPublish) => {
+    t.deepEqual(params, {
+      author: '[email protected]',
+      currentAuthor: currentAuthor,
+      filePath: file,
+      ownedLines: 1,
+      size: 1,
+      threshold: config.threshold,
+    });
+    t.end();
+  });
+
+  processFile({ file, excludeUsers, publish, currentAuthor, config });
+});
+
+test.cb('calls checkOwnership with comments and multiple authors', t => {
+  t.plan(2);
+
+  const blameResult = fs.readFileSync('fixtures/blame-multiple-comment.txt', 'utf8');
+
+  const checkOwnershipStub = sandbox.stub(utils, 'checkOwnership');
+  const file = 'fixtures/code-comment-single-line.js';
+  sandbox.stub(cp, 'exec').withArgs(`git blame -w --show-email HEAD~1 -- ${file}`).yields(undefined, blameResult, undefined);
+  const excludeUsers = [];
+  const publish = false;
+  const currentAuthor = '[email protected]';
+  const config = { threshold: 42, exclude: { comments: {'.js': '//'} } };
+
+  checkOwnershipStub.onCall(0).callsFake((params, givenPublish) => {
+    t.deepEqual(params, {
+      author: '[email protected]',
+      currentAuthor: currentAuthor,
+      filePath: file,
+      ownedLines: 2,
+      size: 3,
+      threshold: config.threshold,
+    });
+  });
+  checkOwnershipStub.onCall(1).callsFake((params, givenPublish) => {
+    t.deepEqual(params, {
+      author: '[email protected]',
+      currentAuthor: currentAuthor,
+      filePath: file,
+      ownedLines: 1,
+      size: 3,
+      threshold: config.threshold,
+    });
+    t.end();
+  });
+
+  processFile({ file, excludeUsers, publish, currentAuthor, config });
+});

modules/read-config.js

@@ -14,8 +14,12 @@ module.exports = (location = '.pythia-config') => {
     }
   });
 
+  if (typeof exclude.comments === 'undefined') {
+    exclude.comments = {};
+  }
+
   return {
   	exclude,
     threshold
   };
-};
+};