Skip to content

Latest commit

 

History

History
1373 lines (1024 loc) · 27.1 KB

STYLE_GUIDE.md

File metadata and controls

1373 lines (1024 loc) · 27.1 KB
Raw

Style Guide

Having a consistent style across all specs helps make them easier for a reader to understand at a glance and easier for an author to focus on the behavior being tested rather than the details of how the test is written. This guide sets out the standard style that should be used for all new specs.

Note that specs written before this style guide probably don't follow it exactly (or at all). If you're inspired to fix up old specs, you're very welcome to do so!

Each guideline starts with one of these words:

  • DO guidelines describe practices that should always be followed. There will almost never be a valid reason to stray from them.

  • DON'T guidelines are the converse: things that are almost never a good idea.

  • PREFER guidelines are practices that you should follow. However, there may be circumstances where it makes sense to do otherwise. Just make sure you understand the full implications of ignoring the guideline when you do.

  • AVOID guidelines are the dual to "prefer": stuff you shouldn't do but where there may be good reasons to on rare occasions.

  • CONSIDER guidelines are practices that you might or might not want to follow, depending on circumstances, precedents, and your own preference.

That specs that are testing inputs that violate these guidelines are, of course, an exception. Thorough testing trumps style.

Organization

DO write specs in HRX files

Example

Good

nesting.hrx

<===> single/input.scss
a {
  b {c: d}
}

<===> single/output.css
a b {
  c: d;
}

Bad

nesting/single/input.scss

a {
  b {c: d}
}

nesting/single/output.css

a b {
  c: d;
}

All specs should be written in HRX files unless they contain invalid UTF-8 characters. HRX files make it easy for code reviewers to see the context of specs they're reviewing.

DO use as few HRX files as possible for a given feature

Example

Good

nesting.hrx

<===> single/input.scss
a {
  b {c: d}
}

<===> single/output.css
a b {
  c: d;
}

<===>
================================================================================
<===> double/input.scss
a {
  b, c {d: e}
}

<===> double/output.css
a b, a c {
  d: e;
}

Bad

nesting/single.hrx

<===> input.scss
a {
  b {c: d}
}

<===> output.css
a b {
  c: d;
}

nesting/double.hrx

<===> input.scss
a {
  b, c {d: e}
}

<===> output.css
a b, a c {
  d: e;
}

Combining specs into as few HRX files as possible helps keep the repo clean and easy to navigate, make it easier to see connections between related specs, and encourages the creation of more smaller specs.

Note that specs should not be combined for unrelated features, and that HRX files should be split apart if they become too large (see below).

DON'T have HRX files longer than 500 lines or so

Very large HRX files can be as difficult to navigate as hundreds of tiny files. Once an HRX file gets larger than about 500 lines, it's probably time to split it into multiple files. To do so:

  • Create a directory with the same name as the HRX files.
  • Take all the top-level subdirectories in the HRX file and make them their own HRX files.
  • If this creates too many small HRX files, consider combining some related files back into more coarse-grained sub-divisions.

DO organize specs by which part of the language they test

Example

Good

css/directives/keyframes.hrx

<===> bubble/empty/input.scss
// Regression test for sass/dart-sass#611.
a {
  @keyframes {/**/}
}

<===> bubble/empty/output.css
@keyframes {
  /**/
}

Bad

regression/dart_sass_611.hrx

<===> input.scss
a {
  @keyframes {/**/}
}

<===> output.css
@keyframes {
  /**/
}

Specs should be organized into hierarchical directories based on what part of the language they test, rather than other factors like how the test came to exist, which implementations support it, and so on. If a spec covers the intersection of multiple features, it can go in any of those features' locations, based on which the spec writer feels most clearly communicates the spec's meaning.

The following directories should be used for their corresponding language features:

  • spec/css/ is for plain CSS features, including CSS at-rules like @media and plain-CSS selector syntax.

  • spec/core_functions/ is for built-in Sass functions. These functions should go in directories whose names correspond to the modules in the module system proposal. For example, specs for the rgb() function go in spec/core_functions/color/rgb/.

  • spec/directives is for Sass's at-rules like @extend and @import.

  • spec/values/ is for SassScript value types.

DO use descriptive paths

Example

Good

css/keyframes.hrx

<===> bubble/empty/input.scss
// Regression test for sass/dart-sass#611.
a {
  @keyframes {/**/}
}

<===> bubble/empty/output.css
@keyframes {
  /**/
}

Bad

basic/edge_case.hrx

<===> input.scss
a {
  @keyframes {/**/}
}

<===> output.css
@keyframes {
  /**/
}

The path to a spec, both inside and outside its HRX file, is the first place a reader will look to try to understand exactly what it's testing. Being as descriptive as possible (without being too verbose) in your choice of names will make this understanding much easier. Good rules of thumb include:

  • Use noun phrases (like "keyframes") or adjectives (like "empty") that describe the feature being tested or the context or state it's in.

  • Avoid placeholder names like "basic" or "simple". Sometimes it's best to describe how it's basic, like "empty" or "one_child". Other times, you can just forego the "basic" directory entirely and put your specs beneath the feature itself.

If you can't fully and tersely describe a spec with its name alone, add a comment to the beginning of the Sass file.

DO put error specs in a separate "error" directory

Example

Good

<===> single_value/input.scss
a {
  --b: c;
}

<===> single_value/output.css
a {
  --b: c;
}

<===>
================================================================================
<===> error/empty/input.scss
// CSS requires at least one token in a custom property.
.empty {
  --property:;
}

<===> error/empty/error
Error: Expected token.
  ,
3 |   --property:;
  |              ^
  '
  /sass/spec/css/custom_properties/error/empty/input.scss 3:14  root stylesheet

Bad

<===> single_value/input.scss
a {
  --b: c;
}

<===> single_value/output.css
a {
  --b: c;
}

<===>
================================================================================
<===> empty/input.scss
// CSS requires at least one token in a custom property.
.empty {
  --property:;
}

<===> empty/error
Error: Expected token.
  ,
3 |   --property:;
  |              ^
  '
  /sass/spec/css/custom_properties/error/empty/input.scss 3:14  root stylesheet

All error specs for a given feature should go in an error directory (which is often its own HRX file) directly beneath the root directory for the feature being tested. They should not be intermingled with the feature's success case specs. This helps keep the success specs focused on demonstrating the expected behavior of the feature, while the error tests can cover all the disallowed edge cases without getting in the way.

PREFER underscore-separated paths

Example

Good

css/min_max.hrx

<===> plain_css/nested_min_max/input.scss
a {
  b: min(max(1px, 2px)) max(min(1px, 2px));
}

<===> plain_css/nested_min_max/output.css
a {
  b: min(max(1px, 2px)) max(min(1px, 2px));
}

L#### Bad

css/minMax.hrx

<===> plain-css/nested-min-max/input.scss
a {
  b: min(max(1px, 2px)) max(min(1px, 2px));
}

<===> plain-css/nested-min-max/output.css
a {
  b: min(max(1px, 2px)) max(min(1px, 2px));
}

Path names should be underscore-separated rather than hyphenated, space-separated, or camel-cased. The only exception is when the path is describing an existing Sass or CSS identifier that contains hyphens, such as font-face or scale-color.

Testing

DO test only one thing per spec

Example

Good

<===> transparent/input.scss
a {b: hsl(180, 60%, 50%, 0)}

<===> transparent/output.css
a {
  b: rgba(51, 204, 204, 0);
}

<===>
================================================================================
<===> opaque/input.scss
a {b: hsl(180, 60%, 50%, 1)}

<===> opaque/output.css
a {
  b: #33cccc;
}

Bad

<===> input.scss
a {
  transparent: hsl(180, 60%, 50%, 0);
  opaque: hsl(180, 60%, 50%, 1);
}

<===> output.css
a {
  transparent: hsl(180, 60%, 50%, 1);
  opaque: #33cccc;
}

Each individual spec should focus on exactly one assertion of the behavior in question. It's worth a bit of extra verbosity to ensure that it's always clear exactly what each spec is testing. Otherwise, it's extremely difficult to refactor specs because you don't know which of the dozen behaviors might not be tested anywhere else. It also ensures that an implementation can mark an individual assertion as :todo without also ceasing to test a bunch of unrelated behavior.

DO use type selectors as placeholders

Example

Good

<===> transparent/input.scss
a {b: hsl(180, 60%, 50%, 0)}

<===> transparent/output.css
a {
  b: rgba(51, 204, 204, 0);
}

Bad

<===> transparent/input.scss
.a {b: hsl(180, 60%, 50%, 0)}

<===> transparent/output.css
.a {
  b: rgba(51, 204, 204, 0);
}

When you need a style rule as context for something else, like a test for an expression, the kind of selector you use for it doesn't matter. It's just there as a placeholder to make the style rule valid. In that case, use a type selector like a to avoid any unnecessary punctuation.

DO use descriptive names for multiple placeholders

Example

Good

<===> input.scss
extendee {a: b}

extender {
  @extend extendee;
}

<===> output.css
extendee, extender {
  a: b;
}

Bad

<===> input.scss
a {b: c}

d {
  @extend a;
}

<===> output.css
a, d {
  b: c;
}

If you have more than one placeholder selector (or more than one placeholder property, etc) in the same test, give them explicit names to help make it clear how the input maps to the output. These names should describe the role of the placeholders (or their associated rules), or at least their original locations.

DO use single-letter names for irrelevant placeholders

Example

Good

<===> input.scss
a {b: c}

<===> output.css
a {
  b: c;
}

Bad

<===> input.scss
@function global-function() {@return null}

global-function-exists {
  result: function-exists(global-function);
}

<===> output.css
global-function-exists {
  result: true;
}

If your placeholder identifiers aren't repeated, use single-letter names in alphabetical order for them. The purpose of the spec should already be communicated by its path, and it should only be testing one thing, so there should be no need to add extra description to a single selector or property name.

DO use pfx for vendor prefixes

Example

Good

<===> input.scss
a {b: is-superselector(":-pfx-matches(c d)", "c d")}

<===> output.css
a {
  b: true;
}

Bad

<===> input.scss
a {b: is-superselector(":-webkit-matches(c d)", "c d")}

<===> output.css
a {
  b: true;
}

There are a few situation in which Sass specifically parses vendor prefixes. In these cases, all vendor prefixes should be treated equivalently, whether they're from a real browser or not. To test this, use the non-existent vendor prefix pfx.

If multiple different vendor prefixes are needed, index them by letter, as -pfxa-, -pfxb, and so on.

DO use style rules for expression-level tests

Example

Good

<===> transparent/input.scss
a {b: hsl(180, 60%, 50%, 0)}

<===> transparent/output.css
a {
  b: rgba(51, 204, 204, 0);
}

Bad

<===> transparent/input.scss
@debug hsl(180, 60%, 50%, 0);

<===> transparent/output.css
<===> transparent/warning
/spec/core_functions/hsl/transparent/input.scss:1 Debug: rgba(51, 204, 204, 0)

When you're testing something at the expression level, like a function or an arithmetic operation, write the expression as the right-hand side of a declaration in a style rule. This will produce as little visual noise as possible without tying your spec to the implementation-specific formatting of @warn or @debug.

PREFER making imported files partials

Example

Good

<===> member/variable/input.scss
@import "other";

a {b: $c}

<===> member/variable/_other.scss
$c: d;

<===> member/variable/output.scss
a {
  b: d;
}

Bad

<===> member/variable/input.scss
@import "other";

a {b: $c}

<===> member/variable/other.scss
$c: d;

<===> member/variable/output.scss
a {
  b: d;
}

When writing a spec with multiple files, make the files (other than input.scss or input.sass) partials unless there's a concrete reason not to. This helps make it clearer at a glance that they're additional input files, rather than expectation files.

DO name single imported or used files "other"

Example

Good

<===> member/variable/input.scss
@import "other";

a {b: $c}

<===> member/variable/_other.scss
$c: d;

<===> member/variable/output.scss
a {
  b: d;
}

Bad

<===> member/variable/input.scss
@import "imported";

a {b: $c}

<===> member/variable/_imported.scss
$c: d;

<===> member/variable/output.scss
a {
  b: d;
}

When a spec involves exactly two files, the file that's not input.scss should be named "other" (_other.scss, _other.sass, etc).

PREFER referring to issues when marking specs as :todo

Example

Good

<===> slash_slash_string/options.yml
---
:todo:
- sass/dart-sass#123456

<===> slash_slash_string/input.scss
a {b: 1 / 2 / foo()}

<===> slash_slash_string/output.css
a {
  b: 1/2/foo();
}

Bad

<===> slash_slash_string/options.yml
---
:todo:
- dart-sass

<===> slash_slash_string/input.scss
a {b: 1 / 2 / foo()}

<===> slash_slash_string/output.css
a {
  b: 1/2/foo();
}

When marking a spec as :todo, use a reference to an issue in the implementation in question tracking the feature the spec tests. If there isn't an issue yet, create one. Make sure to also link that issue back to the specs for it!

Documentation

CONSIDER adding a comment explaining your spec

Example

Good

<===> empty/input.scss
// CSS requires at least one token in a custom property.
.empty {
  --property:;
}

<===> empty/error
Error: Expected token.
  ,
3 |   --property:;
  |              ^
  '
  /sass/spec/css/custom_properties/error/empty/input.scss 3:14  root stylesheet

If the details of what your spec is testing and why it produces the output it does wouldn't be clear to someone moderately familiar with Sass from the spec's path and a casual reading of the spec's contents, it's a good idea to add a comment explaining in detail what's going on. This can also be useful for elucidating the stranger corners of the CSS spec or Sass's language semantics.

Explanatory comments should always be silent Sass comments. Loud comments and HRX comments must not be used.

DON'T write HRX comments

Example

Bad

<===>
CSS requires at least one token in a custom property.
<===> empty/input.scss
.empty {
  --property:;
}

<===> empty/error
Error: Expected token.
  ,
3 |   --property:;
  |              ^
  '
  /sass/spec/css/custom_properties/error/empty/input.scss 3:14  root stylesheet

HRX supports its own comment syntax, but that should never be used for specs. Instead use Sass comments to document an individual file or spec and README.md files to document entire directories. Sass comments provide more flexibility to provide documentation for specific portions of the test files.

CONSIDER adding README.md files to parent directories

Example

Good

<===> color/literal/README.md
When colors are written literally by the user, rather than being generated by a
function, `inspect()` should return them in the same format the user wrote them.

<===>
================================================================================
<===> color/literal/short_hex/input.scss
a {
  $result: inspect(#00f);
  result: $result;
  type: type-of($result);
}

<===> color/literal/short_hex/output.css
a {
  result: #00f;
  type: string;
}

<===>
================================================================================
<===> color/literal/long_hex/input.scss
a {
  $result: inspect(#0000ff);
  result: $result;
  type: type-of($result);
}

<===> color/literal/long_hex/output.css
a {
  result: #0000ff;
  type: string;
}

Like adding a comment to an individual spec, add a README.md file to a directory to explain something about all the specs that directory contains. There's not currently a way to render the Markdown, but it provides a consistent format that's easy to read in plain text.

Formatting

DO use comment dividers to separate specs within an HRX file

Example

Good

<===> single/input.scss
a {
  b {c: d}
}

<===> single/output.css
a b {
  c: d;
}

<===>
================================================================================
<===> double/input.scss
a {
  b, c {d: e}
}

<===> double/output.css
a b, a c {
  d: e;
}

Bad

<===> single/input.scss
a {
  b {c: d}
}

<===> single/output.css
a b {
  c: d;
}

<===> double/input.scss
a {
  b, c {d: e}
}

<===> double/output.css
a b, a c {
  d: e;
}

Each subdirectory within an HRX file after the first should begin with an HRX comment containing 80 = characters:

<===>
================================================================================

Because this is an HRX comment, it has no effect on the contents of the directory. It just serves to visually separate specs from one another.

DO order files consistently

Example

Good

<===> top_level/options.yml
---
:warning_todo:
- sass/dart-sass#123456
:ignore_for:
- ruby-sass

<===> top_level/input.scss
$var: value !global;
a {b: $var}

<===> top_level/output.css
a {
  b: value;
}

<===> top_level/warning
DEPRECATION WARNING: As of Dart Sass 2.0.0, !global assignments won't be able to
declare new variables. Consider adding `$var: null` at the top level.

  ,
1 | $var: value !global;
  | ^^^^^^^^^^^^^^^^^^^
  '
    /sass/spec/variables/global/first_declaration/top_level/input.scss 1:1  root stylesheet

Bad

<===> top_level/output.css
a {
  b: value;
}

<===> top_level/warning
DEPRECATION WARNING: As of Dart Sass 2.0.0, !global assignments won't be able to
declare new variables. Consider adding `$var: null` at the top level.

  ,
1 | $var: value !global;
  | ^^^^^^^^^^^^^^^^^^^
  '
    /sass/spec/variables/global/first_declaration/top_level/input.scss 1:1  root stylesheet

<===> top_level/options.yml
---
:warning_todo:
- sass/dart-sasss#123456
:ignore_for:
- ruby-sass

<===> top_level/input.scss
$var: value !global;
a {b: $var}

k

To keep specs consistent, their constituent files should always be written in the following order:

  1. options.yml.
  2. input.scss or input.sass.
  3. Any files imported or used by input.scss or input.sass.
  4. output.css or error
  5. warning

If a spec has implementation-specific expectations, those expectations should go directly after the corresponding default expectation.

DO use Dart Sass's output as the default

Example

Good

<===> single_channel/input.scss
a {b: complement(#f00)}

<===> single_channel/output.css
a {
  b: aqua;
}

<===> single_channel/output-libsass.css
a {
  b: cyan;
}

Bad

<===> single_channel/input.scss
a {b: complement(#f00)}

<===> single_channel/output.css
a {
  b: cyan;
}

<===> single_channel/output-dart-sass.css
a {
  b: aqua;
}

Since Dart Sass is the reference implementation, its output should be used as output.css, and other implementations' outputs (if they differ) should be written as implementation-specific expectations.

DO end each file with a newline

Example

Good

<===> single/input.scss
a {
  b {c: d}
}

<===> single/output.css
a b {
  c: d;
}

Bad

<===> single/input.scss
a {
  b {c: d}
}
<===> single/output.css
a b {
  c: d;
}

Unless the lack of a trailing newline is specifically being tested, all non-empty files should end in a trailing newline. For all but the last file in an HRX archive, this appears as a full blank line.

DO use two spaces for indentation

Example

Good

<===> single/input.scss
a {
  b {c: d}
}

<===> single/output.css
a b {
  c: d;
}

Bad

<===> single/input.scss
a {
	b {c: d}
}

<===> single/output.css
a b {
  c: d;
}

Always use two spaces for indentation unless you're explicitly testing behavior for other whitespace.

DO use one-line rules when they contain empty children

Example

Good

<===> empty/input.scss
a {}

<===> empty/output.css

Bad

<===> empty/input.scss
a {
}

<===> empty/output.css

PREFER one-line rules when they contain a single child with no children

Example

Good

<===> single/input.scss
a {
  b {c: d}
}

<===> single/output.css
a b {
  c: d;
}

Bad

<===> single/input.scss
a {
  b {
    c: d;
  }
}

<===> single/output.css
a b {
  c: d;
}

When a rule contains a single child statement, such as a style rule that contains a single declaration or a @function that just returns a value, it should generally be written as a single line with no whitespace after { or before }. However, if this would produce too long of a line or otherwise make the rule difficult to read, it may be split onto multiple lines.

DON'T use one-line rules when they contain more than one child

Example

Good

<===> two_declarations/input.scss
a {
  b: c;
  d: e;
}

<===> two_declarations/output.css
a {
  b: c;
  d: e;
}

Bad

<===> two_declarations/input.scss
a {b: c; d: e}

<===> two_declarations/output.css
a {
  b: c;
  d: e;
}

When a rule contains multiple child statements, each child must be written on its own line. Each child that doesn't take a block must be followed by a semicolon.

DON'T use one-line rules when they contain grandchildren

Example

Good

<===> single/input.scss
a {
  b {c: d}
}

<===> single/output.css
a b {
  c: d;
}

Bad

<===> single/input.scss
a {b {c: d}}

<===> single/output.css
a b {
  c: d;
}

If a rule contains another rule which itself has children, the grandparent should never be written on a single line.