diff --git a/config/src/main/java/com/typesafe/config/impl/Parser.java b/config/src/main/java/com/typesafe/config/impl/Parser.java index 13331d35..94c6189a 100644 --- a/config/src/main/java/com/typesafe/config/impl/Parser.java +++ b/config/src/main/java/com/typesafe/config/impl/Parser.java @@ -41,14 +41,26 @@ final class Parser { TokenWithComments(Token token, List comments) { this.token = token; this.comments = comments; + + if (Tokens.isComment(token)) + throw new ConfigException.BugOrBroken("tried to annotate a comment with a comment"); } TokenWithComments(Token token) { this(token, Collections. emptyList()); } + TokenWithComments removeAll() { + if (comments.isEmpty()) + return this; + else + return new TokenWithComments(token); + } + TokenWithComments prepend(List earlier) { - if (this.comments.isEmpty()) { + if (earlier.isEmpty()) { + return this; + } else if (this.comments.isEmpty()) { return new TokenWithComments(token, earlier); } else { List merged = new ArrayList(); @@ -58,7 +70,18 @@ final class Parser { } } - SimpleConfigOrigin setComments(SimpleConfigOrigin origin) { + TokenWithComments add(Token after) { + if (this.comments.isEmpty()) { + return new TokenWithComments(token, Collections. singletonList(after)); + } else { + List merged = new ArrayList(); + merged.addAll(comments); + merged.add(after); + return new TokenWithComments(token, merged); + } + } + + SimpleConfigOrigin prependComments(SimpleConfigOrigin origin) { if (comments.isEmpty()) { return origin; } else { @@ -66,7 +89,19 @@ final class Parser { for (Token c : comments) { newComments.add(Tokens.getCommentText(c)); } - return origin.setComments(newComments); + return origin.prependComments(newComments); + } + } + + SimpleConfigOrigin appendComments(SimpleConfigOrigin origin) { + if (comments.isEmpty()) { + return origin; + } else { + List newComments = new ArrayList(); + for (Token c : comments) { + newComments.add(Tokens.getCommentText(c)); + } + return origin.appendComments(newComments); } } @@ -105,12 +140,38 @@ final class Parser { this.equalsCount = 0; } + static private boolean attractsTrailingComments(Token token) { + // END can't have a trailing comment; START, OPEN_CURLY, and + // OPEN_SQUARE followed by a comment should behave as if the comment + // went with the following field or element. Associating a comment + // with a newline would mess up all the logic for comment tracking, + // so don't do that either. + if (Tokens.isNewline(token) || token == Tokens.START || token == Tokens.OPEN_CURLY + || token == Tokens.OPEN_SQUARE || token == Tokens.END) + return false; + else + return true; + } + + static private boolean attractsLeadingComments(Token token) { + // a comment just before a close } generally doesn't go with the + // value before it, unless it's on the same line as that value + if (Tokens.isNewline(token) || token == Tokens.START || token == Tokens.CLOSE_CURLY + || token == Tokens.CLOSE_SQUARE || token == Tokens.END) + return false; + else + return true; + } + private void consolidateCommentBlock(Token commentToken) { // a comment block "goes with" the following token // unless it's separated from it by a blank line. // we want to build a list of newline tokens followed // by a non-newline non-comment token; with all comments // associated with that final non-newline non-comment token. + // a comment AFTER a token, without an intervening newline, + // also goes with that token, but isn't handled in this method, + // instead we handle it later by peeking ahead. List newlines = new ArrayList(); List comments = new ArrayList(); @@ -128,6 +189,11 @@ final class Parser { comments.add(next); } else { // a non-newline non-comment token + + // comments before a close brace or bracket just get dumped + if (!attractsLeadingComments(next)) + comments.clear(); + break; } @@ -146,7 +212,7 @@ final class Parser { } } - private TokenWithComments popToken() { + private TokenWithComments popTokenWithoutTrailingComment() { if (buffer.isEmpty()) { Token t = tokens.next(); if (Tokens.isComment(t)) { @@ -160,6 +226,35 @@ final class Parser { } } + private TokenWithComments popToken() { + TokenWithComments withPrecedingComments = popTokenWithoutTrailingComment(); + // handle a comment AFTER the other token, + // but before a newline. If the next token is not + // a comment, then any comment later on the line is irrelevant + // since it would end up going with that later token, not + // this token. Comments are supposed to be processed prior + // to adding stuff to the buffer, so they can only be found + // in "tokens" not in "buffer" in theory. + if (!attractsTrailingComments(withPrecedingComments.token)) { + return withPrecedingComments; + } else if (buffer.isEmpty()) { + Token after = tokens.next(); + if (Tokens.isComment(after)) { + return withPrecedingComments.add(after); + } else { + buffer.push(new TokenWithComments(after)); + return withPrecedingComments; + } + } else { + // comments are supposed to get attached to a token, + // not put back in the buffer. Assert this as an invariant. + if (Tokens.isComment(buffer.peek().token)) + throw new ConfigException.BugOrBroken( + "comment token should not have been in buffer: " + buffer); + return withPrecedingComments; + } + } + private TokenWithComments nextToken() { TokenWithComments withComments = null; @@ -192,6 +287,9 @@ final class Parser { } private void putBack(TokenWithComments token) { + if (Tokens.isComment(token.token)) + throw new ConfigException.BugOrBroken( + "comment token should have been stripped before it was available to put back"); buffer.push(token); } @@ -216,6 +314,19 @@ final class Parser { return t; } + private AbstractConfigValue addAnyCommentsAfterAnyComma(AbstractConfigValue v) { + TokenWithComments t = nextToken(); // do NOT skip newlines, we only + // want same-line comments + if (t.token == Tokens.COMMA) { + // steal the comments from after the comma + putBack(t.removeAll()); + return v.withOrigin(t.appendComments(v.origin())); + } else { + putBack(t); + return v; + } + } + // In arrays and objects, comma can be omitted // as long as there's at least one newline instead. // this skips any newlines in front of a comma, @@ -272,22 +383,14 @@ final class Parser { // create only if we have value tokens List values = null; - TokenWithComments firstValueWithComments = null; + // ignore a newline up front TokenWithComments t = nextTokenIgnoringNewline(); while (true) { AbstractConfigValue v = null; - if (Tokens.isValue(t.token)) { - // if we consolidateValueTokens() multiple times then - // this value could be a concatenation, object, array, - // or substitution already. - v = Tokens.getValue(t.token); - } else if (Tokens.isUnquotedText(t.token)) { - v = new ConfigString(t.token.origin(), Tokens.getUnquotedText(t.token)); - } else if (Tokens.isSubstitution(t.token)) { - v = new ConfigReference(t.token.origin(), - tokenToSubstitutionExpression(t.token)); - } else if (t.token == Tokens.OPEN_CURLY || t.token == Tokens.OPEN_SQUARE) { + if (Tokens.isValue(t.token) || Tokens.isUnquotedText(t.token) + || Tokens.isSubstitution(t.token) || t.token == Tokens.OPEN_CURLY + || t.token == Tokens.OPEN_SQUARE) { // there may be newlines _within_ the objects and arrays v = parseValue(t); } else { @@ -299,7 +402,6 @@ final class Parser { if (values == null) { values = new ArrayList(); - firstValueWithComments = t; } values.add(v); @@ -313,11 +415,10 @@ final class Parser { AbstractConfigValue consolidated = ConfigConcatenation.concatenate(values); - putBack(new TokenWithComments(Tokens.newValue(consolidated), - firstValueWithComments.comments)); + putBack(new TokenWithComments(Tokens.newValue(consolidated))); } - private ConfigOrigin lineOrigin() { + private SimpleConfigOrigin lineOrigin() { return ((SimpleConfigOrigin) baseOrigin).setLineNumber(lineNumber); } @@ -403,7 +504,14 @@ final class Parser { AbstractConfigValue v; if (Tokens.isValue(t.token)) { + // if we consolidateValueTokens() multiple times then + // this value could be a concatenation, object, array, + // or substitution already. v = Tokens.getValue(t.token); + } else if (Tokens.isUnquotedText(t.token)) { + v = new ConfigString(t.token.origin(), Tokens.getUnquotedText(t.token)); + } else if (Tokens.isSubstitution(t.token)) { + v = new ConfigReference(t.token.origin(), tokenToSubstitutionExpression(t.token)); } else if (t.token == Tokens.OPEN_CURLY) { v = parseObject(true); } else if (t.token == Tokens.OPEN_SQUARE) { @@ -413,7 +521,7 @@ final class Parser { "Expecting a value but got wrong token: " + t.token)); } - v = v.withOrigin(t.setComments(v.origin())); + v = v.withOrigin(t.prependComments(v.origin())); return v; } @@ -601,7 +709,7 @@ final class Parser { private AbstractConfigObject parseObject(boolean hadOpenCurly) { // invoked just after the OPEN_CURLY (or START, if !hadOpenCurly) Map values = new HashMap(); - ConfigOrigin objectOrigin = lineOrigin(); + SimpleConfigOrigin objectOrigin = lineOrigin(); boolean afterComma = false; Path lastPath = null; boolean lastInsideEquals = false; @@ -616,6 +724,9 @@ final class Parser { throw parseError(addQuoteSuggestion(t.toString(), "unbalanced close brace '}' with no open brace")); } + + objectOrigin = t.appendComments(objectOrigin); + break; } else if (t.token == Tokens.END && !hadOpenCurly) { putBack(t); @@ -652,8 +763,11 @@ final class Parser { consolidateValueTokens(); valueToken = nextTokenIgnoringNewline(); + // put comments from separator token on the value token + valueToken = valueToken.prepend(afterKey.comments); } + // comments from the key token go to the value token newValue = parseValue(valueToken.prepend(keyToken.comments)); if (afterKey.token == Tokens.PLUS_EQUALS) { @@ -667,6 +781,8 @@ final class Parser { newValue = ConfigConcatenation.concatenate(concat); } + newValue = addAnyCommentsAfterAnyComma(newValue); + lastPath = pathStack.pop(); if (insideEquals) { equalsCount -= 1; @@ -722,6 +838,9 @@ final class Parser { throw parseError(addQuoteSuggestion(lastPath, lastInsideEquals, t.toString(), "unbalanced close brace '}' with no open brace")); } + + objectOrigin = t.appendComments(objectOrigin); + break; } else if (hadOpenCurly) { throw parseError(addQuoteSuggestion(lastPath, lastInsideEquals, @@ -743,7 +862,7 @@ final class Parser { private SimpleConfigList parseArray() { // invoked just after the OPEN_SQUARE - ConfigOrigin arrayOrigin = lineOrigin(); + SimpleConfigOrigin arrayOrigin = lineOrigin(); List values = new ArrayList(); consolidateValueTokens(); @@ -752,11 +871,13 @@ final class Parser { // special-case the first element if (t.token == Tokens.CLOSE_SQUARE) { - return new SimpleConfigList(arrayOrigin, + return new SimpleConfigList(t.appendComments(arrayOrigin), Collections. emptyList()); } else if (Tokens.isValue(t.token) || t.token == Tokens.OPEN_CURLY || t.token == Tokens.OPEN_SQUARE) { - values.add(parseValue(t)); + AbstractConfigValue v = parseValue(t); + v = addAnyCommentsAfterAnyComma(v); + values.add(v); } else { throw parseError(addKeyName("List should have ] or a first element after the open [, instead had token: " + t @@ -773,7 +894,7 @@ final class Parser { } else { t = nextTokenIgnoringNewline(); if (t.token == Tokens.CLOSE_SQUARE) { - return new SimpleConfigList(arrayOrigin, values); + return new SimpleConfigList(t.appendComments(arrayOrigin), values); } else { throw parseError(addKeyName("List should have ended with ] or had a comma, instead had token: " + t @@ -789,7 +910,9 @@ final class Parser { t = nextTokenIgnoringNewline(); if (Tokens.isValue(t.token) || t.token == Tokens.OPEN_CURLY || t.token == Tokens.OPEN_SQUARE) { - values.add(parseValue(t)); + AbstractConfigValue v = parseValue(t); + v = addAnyCommentsAfterAnyComma(v); + values.add(v); } else if (flavor != ConfigSyntax.JSON && t.token == Tokens.CLOSE_SQUARE) { // we allow one trailing comma putBack(t); diff --git a/config/src/main/java/com/typesafe/config/impl/SimpleConfigOrigin.java b/config/src/main/java/com/typesafe/config/impl/SimpleConfigOrigin.java index 353e9c05..7af380aa 100644 --- a/config/src/main/java/com/typesafe/config/impl/SimpleConfigOrigin.java +++ b/config/src/main/java/com/typesafe/config/impl/SimpleConfigOrigin.java @@ -93,6 +93,34 @@ final class SimpleConfigOrigin implements ConfigOrigin { } } + SimpleConfigOrigin prependComments(List comments) { + if (ConfigImplUtil.equalsHandlingNull(comments, this.commentsOrNull) || comments == null) { + return this; + } else if (this.commentsOrNull == null) { + return setComments(comments); + } else { + List merged = new ArrayList(comments.size() + + this.commentsOrNull.size()); + merged.addAll(comments); + merged.addAll(this.commentsOrNull); + return setComments(merged); + } + } + + SimpleConfigOrigin appendComments(List comments) { + if (ConfigImplUtil.equalsHandlingNull(comments, this.commentsOrNull) || comments == null) { + return this; + } else if (this.commentsOrNull == null) { + return setComments(comments); + } else { + List merged = new ArrayList(comments.size() + + this.commentsOrNull.size()); + merged.addAll(this.commentsOrNull); + merged.addAll(comments); + return setComments(merged); + } + } + @Override public String description() { // not putting the URL in here for files and resources, because people diff --git a/config/src/test/scala/com/typesafe/config/impl/ConfParserTest.scala b/config/src/test/scala/com/typesafe/config/impl/ConfParserTest.scala index 85da04dd..c865c651 100644 --- a/config/src/test/scala/com/typesafe/config/impl/ConfParserTest.scala +++ b/config/src/test/scala/com/typesafe/config/impl/ConfParserTest.scala @@ -367,6 +367,10 @@ class ConfParserTest extends TestUtils { Parseable.newReader(new StringReader("{}"), options).toString } + private def assertComments(comments: Seq[String], conf: Config) { + assertEquals(comments, conf.root().origin().comments().asScala.toSeq) + } + private def assertComments(comments: Seq[String], conf: Config, path: String) { assertEquals(comments, conf.getValue(path).origin().comments().asScala.toSeq) } @@ -377,17 +381,24 @@ class ConfParserTest extends TestUtils { } @Test - def trackCommentsForFields() { - // comment in front of a field is used - val conf1 = parseConfig(""" - { # Hello + def trackCommentsForSingleField() { + // no comments + val conf0 = parseConfig(""" + { foo=10 } """) - assertComments(Seq(" Hello"), conf1, "foo") + assertComments(Seq(), conf0, "foo") + + // comment in front of a field is used + val conf1 = parseConfig(""" + { # Before + foo=10 } + """) + assertComments(Seq(" Before"), conf1, "foo") // comment with a blank line after is dropped val conf2 = parseConfig(""" - { # Hello + { # BlankAfter foo=10 } """) @@ -395,19 +406,175 @@ class ConfParserTest extends TestUtils { // comment in front of a field is used with no root {} val conf3 = parseConfig(""" - # Hello + # BeforeNoBraces foo=10 """) - assertComments(Seq(" Hello"), conf3, "foo") + assertComments(Seq(" BeforeNoBraces"), conf3, "foo") // comment with a blank line after is dropped with no root {} val conf4 = parseConfig(""" - # Hello + # BlankAfterNoBraces foo=10 """) assertComments(Seq(), conf4, "foo") + // comment same line after field is used + val conf5 = parseConfig(""" + { + foo=10 # SameLine + } + """) + assertComments(Seq(" SameLine"), conf5, "foo") + + // comment before field separator is used + val conf6 = parseConfig(""" + { + foo # BeforeSep + =10 + } + """) + assertComments(Seq(" BeforeSep"), conf6, "foo") + + // comment after field separator is used + val conf7 = parseConfig(""" + { + foo= # AfterSep + 10 + } + """) + assertComments(Seq(" AfterSep"), conf7, "foo") + + // comment on next line is NOT used + val conf8 = parseConfig(""" + { + foo=10 + # NextLine + } + """) + assertComments(Seq(), conf8, "foo") + + // comment before field separator on new line + val conf9 = parseConfig(""" + { + foo + # BeforeSepOwnLine + =10 + } + """) + assertComments(Seq(" BeforeSepOwnLine"), conf9, "foo") + + // comment after field separator on its own line + val conf10 = parseConfig(""" + { + foo= + # AfterSepOwnLine + 10 + } + """) + assertComments(Seq(" AfterSepOwnLine"), conf10, "foo") + + // comments comments everywhere + val conf11 = parseConfig(""" + {# Before + foo + # BeforeSep + = # AfterSepSameLine + # AfterSepNextLine + 10 # AfterValue + # AfterValueNewLine (should NOT be used) + } + """) + assertComments(Seq(" Before", " BeforeSep", " AfterSepSameLine", " AfterSepNextLine", " AfterValue"), conf11, "foo") + + // empty object + val conf12 = parseConfig("""# BeforeEmpty + {} #AfterEmpty + # NewLine + """) + assertComments(Seq(" BeforeEmpty", "AfterEmpty"), conf12) + + // empty array + val conf13 = parseConfig(""" + foo= + # BeforeEmptyArray + [] #AfterEmptyArray + # NewLine + """) + assertComments(Seq(" BeforeEmptyArray", "AfterEmptyArray"), conf13, "foo") + + // array element + val conf14 = parseConfig(""" + foo=[ + # BeforeElement + 10 # AfterElement + ] + """) + assertComments(Seq(" BeforeElement", " AfterElement"), conf14, "foo", 0) + + // field with comma after it + val conf15 = parseConfig(""" + foo=10, # AfterCommaField + """) + assertComments(Seq(" AfterCommaField"), conf15, "foo") + + // element with comma after it + val conf16 = parseConfig(""" + foo=[10, # AfterCommaElement + ] + """) + assertComments(Seq(" AfterCommaElement"), conf16, "foo", 0) + + // field with comma after it but comment isn't on the field's line, so not used + val conf17 = parseConfig(""" + foo=10 + , # AfterCommaFieldNotUsed + """) + assertComments(Seq(), conf17, "foo") + + // element with comma after it but comment isn't on the field's line, so not used + val conf18 = parseConfig(""" + foo=[10 + , # AfterCommaElementNotUsed + ] + """) + assertComments(Seq(), conf18, "foo", 0) + + // comment on new line, before comma, should not be used + val conf19 = parseConfig(""" + foo=10 + # BeforeCommaFieldNotUsed + , + """) + assertComments(Seq(), conf19, "foo") + + // comment on new line, before comma, should not be used + val conf20 = parseConfig(""" + foo=[10 + # BeforeCommaElementNotUsed + , + ] + """) + assertComments(Seq(), conf20, "foo", 0) + + // comment on same line before comma + val conf21 = parseConfig(""" + foo=10 # BeforeCommaFieldSameLine + , + """) + assertComments(Seq(" BeforeCommaFieldSameLine"), conf21, "foo") + + // comment on same line before comma + val conf22 = parseConfig(""" + foo=[10 # BeforeCommaElementSameLine + , + ] + """) + assertComments(Seq(" BeforeCommaElementSameLine"), conf22, "foo", 0) + } + + @Test + def trackCommentsForMultipleFields() { // nested objects val conf5 = parseConfig(""" # Outside @@ -418,58 +585,60 @@ class ConfParserTest extends TestUtils { # two lines baz { # Inner - foo=10 # should be ignored - # This should be ignored too - } ## not used + foo=10 # AfterInner + # This should be ignored + } # AfterMiddle # ignored - } + } # AfterOutside # ignored! """) - assertComments(Seq(" Inner"), conf5, "bar.baz.foo") - assertComments(Seq(" Middle", " two lines"), conf5, "bar.baz") - assertComments(Seq(" Outside"), conf5, "bar") + assertComments(Seq(" Inner", " AfterInner"), conf5, "bar.baz.foo") + assertComments(Seq(" Middle", " two lines", " AfterMiddle"), conf5, "bar.baz") + assertComments(Seq(" Outside", " AfterOutside"), conf5, "bar") // multiple fields val conf6 = parseConfig("""{ # this is not with a field - + # this is field A - a : 10 + a : 10, # this is field B - b : 12 # goes with field C + b : 12 # goes with field B which has no comma # this is field C - c : 14, - + c : 14, # goes with field C after comma + # not used # this is not used # nor is this # multi-line block - + # this is with field D # this is with field D also d : 16 - + # this is after the fields }""") assertComments(Seq(" this is field A"), conf6, "a") - assertComments(Seq(" this is field B"), conf6, "b") - assertComments(Seq(" goes with field C", " this is field C"), conf6, "c") + assertComments(Seq(" this is field B", " goes with field B which has no comma"), conf6, "b") + assertComments(Seq(" this is field C", " goes with field C after comma"), conf6, "c") assertComments(Seq(" this is with field D", " this is with field D also"), conf6, "d") // array val conf7 = parseConfig(""" + # before entire array array = [ # goes with 0 0, # goes with 1 - 1, # with 2 + 1, # with 1 after comma # goes with 2 - 2 + 2 # no comma after 2 # not with anything - ] + ] # after entire array """) assertComments(Seq(" goes with 0"), conf7, "array", 0) - assertComments(Seq(" goes with 1"), conf7, "array", 1) - assertComments(Seq(" with 2", " goes with 2"), conf7, "array", 2) + assertComments(Seq(" goes with 1", " with 1 after comma"), conf7, "array", 1) + assertComments(Seq(" goes with 2", " no comma after 2"), conf7, "array", 2) + assertComments(Seq(" before entire array", " after entire array"), conf7, "array") // properties-like syntax val conf8 = parseConfig(""" @@ -484,6 +653,7 @@ class ConfParserTest extends TestUtils { # a.b comment a.b = 14 a.c = 15 + a.d = 16 # a.d comment # ignored comment """) @@ -492,6 +662,7 @@ class ConfParserTest extends TestUtils { assertComments(Seq(" x.a comment"), conf8, "x.a") assertComments(Seq(" a.b comment"), conf8, "a.b") assertComments(Seq(), conf8, "a.c") + assertComments(Seq(" a.d comment"), conf8, "a.d") // here we're concerned that comments apply only to leaf // nodes, not to parent objects. assertComments(Seq(), conf8, "x")