The test case looks good. I think this is the wrong location for the fix though. The code that tests for whether the attribute starts with a quote mark is

The code you are editing is parsing the attribute value, but you are looking to validate the attribute key.

P.S. thanks for submitting the pull request, it makes it much clearer what's going on.

Actually, since this is a breaking change, we should probably only make it a warning, rather than an error. We can then upgrade it to an error in the next major release.

To do this, the test case gets a little more complex. You can start with something a bit like https://github.com/pugjs/pug/blob/master/packages/pug/test/extends-not-top-level/index.test.js but you will need to do something like:

test('invalid attribute name should log warning', () => {
  const oldWarn = console.warn;
  const warnings = [];
  console.warn = warnings.push.bind(warnings);
  pug.compileFile(
    __dirname + '/index.pug'
  );
  console.warn = oldWarn;
  expect(warnings).toMatchSnapshot();
});

Thanks for replying Forbes, I have a question, how to make a warning? (Tried to look around and only found pug-error). I am missing what kind of warning I should return from the modified statement like below.

if(quoteRe.test(str[i])){ 
   quote = str[i]; 
   this.incrementColumn(1); 
   i++; 
 }else if(characterParser.isPunctuator(str[i]) && str[i] !== ':'){

}

By now this is the test, basically what you did plus the import on top.

const pug = require('../../../');

test('invalid attribute name should log warning', () => {
  const oldWarn = console.warn;
  const warnings = [];
  console.warn = warnings.push.bind(warnings);
  pug.compileFile(
    __dirname + '/invalid-character.pug'
  );
  console.warn = oldWarn;
  expect(warnings).toMatchSnapshot();
});

Thanks as always!

Oh okay I initially thought it was console.warn but thought it couldn't be like that since there was pug-error and was expecting pug-warn, anyway gonna try to make a new pull request with the changes you mentioned.

Oops there must be something I might be doing wrong about the snapshot.

@ForbesLindesay I don't know how to fix the push since there is a problem since the snapshot and the result of test produced by travis are not the same, I think it shouldn't take much time.

Anyway consider to close when the issue is fixed or the push is fixed and sent.

Thanks!

You should also make sure you have the correct dependencies installed by running yarn && yarn bootstrap

Hello Forbes this is really tricky, I added your fix (I tried in the line you mentioned and before the last line, but it made no difference), not sure what's wrong with file path and why when it is compared with the snapshot is not the same, because aparently it is the same path.

There is no change log for this pull request yet.

Create a changelog