Merge pull request #82 from typesafehub/havocp-comment-affiliation

Associate comments after a value with that value, fixes #81
This commit is contained in:
Havoc Pennington 2013-09-19 09:50:39 -07:00
commit e76a54843e
3 changed files with 379 additions and 57 deletions

View File

@ -41,14 +41,26 @@ final class Parser {
TokenWithComments(Token token, List<Token> 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.<Token> emptyList());
}
TokenWithComments removeAll() {
if (comments.isEmpty())
return this;
else
return new TokenWithComments(token);
}
TokenWithComments prepend(List<Token> earlier) {
if (this.comments.isEmpty()) {
if (earlier.isEmpty()) {
return this;
} else if (this.comments.isEmpty()) {
return new TokenWithComments(token, earlier);
} else {
List<Token> merged = new ArrayList<Token>();
@ -58,7 +70,18 @@ final class Parser {
}
}
SimpleConfigOrigin setComments(SimpleConfigOrigin origin) {
TokenWithComments add(Token after) {
if (this.comments.isEmpty()) {
return new TokenWithComments(token, Collections.<Token> singletonList(after));
} else {
List<Token> merged = new ArrayList<Token>();
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<String> newComments = new ArrayList<String>();
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<Token> newlines = new ArrayList<Token>();
List<Token> comments = new ArrayList<Token>();
@ -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<AbstractConfigValue> 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<AbstractConfigValue>();
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<String, AbstractConfigValue> values = new HashMap<String, AbstractConfigValue>();
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<AbstractConfigValue> values = new ArrayList<AbstractConfigValue>();
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.<AbstractConfigValue> 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);

View File

@ -93,6 +93,34 @@ final class SimpleConfigOrigin implements ConfigOrigin {
}
}
SimpleConfigOrigin prependComments(List<String> comments) {
if (ConfigImplUtil.equalsHandlingNull(comments, this.commentsOrNull) || comments == null) {
return this;
} else if (this.commentsOrNull == null) {
return setComments(comments);
} else {
List<String> merged = new ArrayList<String>(comments.size()
+ this.commentsOrNull.size());
merged.addAll(comments);
merged.addAll(this.commentsOrNull);
return setComments(merged);
}
}
SimpleConfigOrigin appendComments(List<String> comments) {
if (ConfigImplUtil.equalsHandlingNull(comments, this.commentsOrNull) || comments == null) {
return this;
} else if (this.commentsOrNull == null) {
return setComments(comments);
} else {
List<String> merged = new ArrayList<String>(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

View File

@ -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")