Skip to content

Commit

Permalink
fix: disallow nested media AtRules (#27)
Browse files Browse the repository at this point in the history 
In an earlier change ( ca302db ) to allow an `AtRule` to be nested inside a `ConditionalAtRule`, a bug was introduced allowing an `AtRule` to be nested inside a non-conditional `AtRule`.  This was never supposed to be supported and has been removed.

* Remove developer no-longer supporting the project
  • Loading branch information
@eperret
eperret committed Jul 22, 2021
1 parent bedaefd commit d9a3835
Show file tree
Hide file tree
Showing 3 changed files with 63 additions and 38 deletions.
4 changes: 2 additions & 2 deletions pom.xml
View file
Expand Up @@ -44,8 +44,8 @@

<developers>
<developer>
<name>Nathan McWilliams</name>
<email>nmcwilliams@salesforce.com</email>
<name>Eric Perret</name>
<email>eperret@salesforce.com</email>
<organization>Salesforce</organization>
<organizationUrl>https://opensource.salesforce.com/</organizationUrl>
</developer>
Expand Down
22 changes: 15 additions & 7 deletions src/main/java/com/salesforce/omakase/plugin/syntax/MediaPlugin.java
View file
Expand Up @@ -45,7 +45,7 @@
import com.salesforce.omakase.plugin.Plugin;

/**
* Refines media query at-rules (@media).
* Refines media query at-rules ({@code @media}).
* <p>
* In custom refiner plugins, you can reuse the logic from this class to parse declarations with the {@link
* #delegateRefinement(AtRule, Grammar, Broadcaster)} method.
Expand Down Expand Up @@ -75,7 +75,9 @@ public void refine(AtRule rule, Grammar grammar, Broadcaster broadcaster) {
// refine the expression (unless it was already done)
if (!rule.expression().isPresent()) {
// must have an expression
if (!rule.rawExpression().isPresent()) throw new ParserException(rule, Message.MEDIA_EXPR);
if (!rule.rawExpression().isPresent()) {
throw new ParserException(rule, Message.MEDIA_EXPR);
}

// parse the media query expression
Source source = new Source(rule.rawExpression().get());
Expand All @@ -87,10 +89,14 @@ public void refine(AtRule rule, Grammar grammar, Broadcaster broadcaster) {
Optional<MediaQueryList> list = interest.one();

// must have found a media query list
if (!list.isPresent()) throw new ParserException(source, Message.DIDNT_FIND_MEDIA_LIST);
if (!list.isPresent()) {
throw new ParserException(source, Message.DIDNT_FIND_MEDIA_LIST);
}

// nothing should be left in the expression content
if (!source.skipWhitepace().eof()) throw new ParserException(source, Message.UNPARSABLE_MEDIA, source.remaining());
if (!source.skipWhitepace().eof()) {
throw new ParserException(source, Message.UNPARSABLE_MEDIA, source.remaining());
}

// broadcast the expression
broadcaster.broadcast(list.get());
Expand All @@ -99,16 +105,18 @@ public void refine(AtRule rule, Grammar grammar, Broadcaster broadcaster) {
// refine the block (unless it was already done)
if (!rule.block().isPresent()) {
// must have a block
if (!rule.rawBlock().isPresent()) throw new ParserException(rule, Message.MEDIA_BLOCK);
if (!rule.rawBlock().isPresent()) {
throw new ParserException(rule, Message.MEDIA_BLOCK);
}

Source source = new Source(rule.rawBlock().get());

QueryableBroadcaster queryable = new QueryableBroadcaster(broadcaster);

// parse the inner statements
Parser statementParser = grammar.parser().statementParser();
Parser ruleParser = grammar.parser().ruleParser();
while (!source.eof()) {
boolean matched = statementParser.parse(source, grammar, queryable);
boolean matched = ruleParser.parse(source, grammar, queryable);
source.skipWhitepace();

// after parsing there should be nothing left in the source
Expand Down
75 changes: 46 additions & 29 deletions src/test/java/com/salesforce/omakase/plugin/syntax/MediaPluginTest.java
View file
Expand Up @@ -26,6 +26,16 @@

package com.salesforce.omakase.plugin.syntax;

import static org.fest.assertions.api.Assertions.assertThat;
import static org.junit.Assert.assertThrows;

import java.io.IOException;
import java.util.Iterator;
import java.util.Optional;

import org.junit.Before;
import org.junit.Test;

import com.salesforce.omakase.Message;
import com.salesforce.omakase.ast.RawSyntax;
import com.salesforce.omakase.ast.Statement;
Expand All @@ -42,28 +52,16 @@
import com.salesforce.omakase.parser.ParserException;
import com.salesforce.omakase.writer.StyleAppendable;
import com.salesforce.omakase.writer.StyleWriter;
import org.junit.Before;
import org.junit.Rule;
import org.junit.Test;
import org.junit.rules.ExpectedException;

import java.io.IOException;
import java.util.Iterator;
import java.util.Optional;

import static org.fest.assertions.api.Assertions.assertThat;

/**
* Unit tests for {@link MediaPlugin}.
*
* @author nmcwilliams
*/
@SuppressWarnings("JavaDoc")
public class MediaPluginTest {
@Rule public final ExpectedException exception = ExpectedException.none();

MediaPlugin plugin;
QueryableBroadcaster broadcaster;
private MediaPlugin plugin;
private QueryableBroadcaster broadcaster;

@Before
public void setup() {
Expand Down Expand Up @@ -95,45 +93,64 @@ public void doesntRefineBlockIfAlreadyRefined() {
public void errorsIfMissingExpression() {
AtRule ar = new AtRule(1, 1, "media", null, new RawSyntax(2, 2, ".class{color:red}"));

exception.expect(ParserException.class);
exception.expectMessage(Message.MEDIA_EXPR);
plugin.refine(ar, new Grammar(), broadcaster);
final ParserException parserException = assertThrows(ParserException.class, () -> plugin.refine(ar, new Grammar(), broadcaster));
assertThat(parserException).hasMessageStartingWith(Message.MEDIA_EXPR);
}

@Test
public void errorsIfDidntFindMediaList() {
AtRule ar = new AtRule(1, 1, "media", new RawSyntax(1, 1, ""), new RawSyntax(2, 2, ""));

exception.expect(ParserException.class);
exception.expectMessage(Message.DIDNT_FIND_MEDIA_LIST);
plugin.refine(ar, new Grammar(), broadcaster);
final ParserException parserException = assertThrows(ParserException.class, () -> plugin.refine(ar, new Grammar(), broadcaster));
assertThat(parserException).hasMessageStartingWith(Message.DIDNT_FIND_MEDIA_LIST);
}

@Test
public void errirsIfUnparsableRemainderInExpression() {
AtRule ar = new AtRule(1, 1, "media", new RawSyntax(1, 1, "all$"), new RawSyntax(2, 2, ".class{color:red}"));

exception.expect(ParserException.class);
exception.expectMessage("Unable to parse");
plugin.refine(ar, new Grammar(), broadcaster);
final ParserException parserException = assertThrows(ParserException.class, () -> plugin.refine(ar, new Grammar(), broadcaster));
assertThat(parserException).hasMessageStartingWith("Unable to parse");
}

@Test
public void errorsIfMissingBlock() {
AtRule ar = new AtRule(1, 1, "media", new RawSyntax(1, 1, "all"), null);

exception.expect(ParserException.class);
exception.expectMessage(Message.MEDIA_BLOCK);
plugin.refine(ar, new Grammar(), broadcaster);
final ParserException parserException = assertThrows(ParserException.class, () -> plugin.refine(ar, new Grammar(), broadcaster));
assertThat(parserException).hasMessageStartingWith(Message.MEDIA_BLOCK);
}

@Test
public void errorsIfUnparsableRemainderInBlock() {
AtRule ar = new AtRule(1, 1, "media", new RawSyntax(1, 1, "all"), new RawSyntax(2, 2, ".class{color:red}$"));

exception.expect(ParserException.class);
exception.expectMessage("Unable to parse");
plugin.refine(ar, new Grammar(), broadcaster);
final ParserException parserException = assertThrows(ParserException.class, () -> plugin.refine(ar, new Grammar(), broadcaster));
assertThat(parserException).hasMessageStartingWith("Unable to parse");
}

@Test
public void errorsIfNextedAtRule() {
AtRule ar = new AtRule(1, 1, "media", new RawSyntax(1, 1, "all"), new RawSyntax(2, 2, "@media only screen {\n\t.class{color:red};\n}"));

final ParserException parserException = assertThrows(ParserException.class, () -> plugin.refine(ar, new Grammar(), broadcaster));
assertThat(parserException).hasMessageStartingWith("Unable to parse");
}

@Test
public void errorsIfNextedAtRule2() {
AtRule ar = new AtRule(1, 1, "media", new RawSyntax(1, 1, "all"), new RawSyntax(2, 2, "@media (max-width: 600px) {\n\t.class{color:red};\n}"));

final ParserException parserException = assertThrows(ParserException.class, () -> plugin.refine(ar, new Grammar(), broadcaster));
assertThat(parserException).hasMessageStartingWith("Unable to parse");
}

@Test
public void errorsIfNextedConditionalAtRule() {
AtRule ar = new AtRule(1, 1, "media", new RawSyntax(1, 1, "all"), new RawSyntax(2, 2, "@if(IE) {\n\t.class{color:red};\n}"));

final ParserException parserException = assertThrows(ParserException.class, () -> plugin.refine(ar, new Grammar(), broadcaster));
assertThat(parserException).hasMessageStartingWith("Unable to parse");
}

@Test
Expand Down

0 comments on commit d9a3835

Please sign in to comment.