feat: add prefer-logical-properties rule

[azat-io]

Prerequisites checklist

• I have read the contributing guidelines.

What is the purpose of this pull request?

Add new rule.

What changes did you make? (Give an overview)

Added new rule for usage logical CSS properties instead of phisical.

Related Issues

#13

Is there anything you'd like reviewers to focus on?

I don't think so.

[nzakas]

Thanks for getting this started. There were some edge cases that were discussed in the original issue:

#13 (comment)
#13 (comment)

Can you describe how you're handling those?

[azat-io]

I don't think we need to have an option to exclude certain properties. Since all properties have roughly the same support by browsers. With the exception of logical units only.

What options would you like to see in the rule?

[nzakas]

I think we need a way for people to opt-out on a case-by-case basis. At a minimum, I think we'd need to be able to do this:

{
    "prefer-logical-properties": ["error", {
        allowedProperties: ["padding-left"],   // don't warn on these
        allowedUnits: ["xyz"]  // don't warn on these
    }
}

[azat-io]

I've added options allowedUnits and allowedProperties ✅.

[nzakas]

Thanks, this is looking good. A couple of additional asks:

1. Can you changed allowedProperties to allowProperties and allowedUnits to allowUnits? I just went through and checked which convention we are using in other rules and it's "allow" rather than "allowed".
2. Can you also update the docs to mention the new options?

[nzakas]

To fix the type error.

Suggested change

	fixable: "code",
	fixable: /** @type {const} */ ("code"),

[azat-io]

Fixed ✅

[azat-io]

Renamed the options and added their descriptions to the documentation.

[nzakas]

Just to finish up the docs, we typically include two additional sections:

1. When Not to Use It - gives guidance on when it's okay to not use the rule
2. Prior Art - links to other tools that do the same thing

Here's an example:
https://github.com/eslint/css/blob/main/docs/rules/no-invalid-properties.md

For prior art, we have:

• https://github.com/csstools/stylelint-use-logical
• https://www.npmjs.com/package/stylelint-plugin-logical-css

[azat-io]

Added ✅

[nzakas]

This is looking good. I just realized that it will also error for @supports rules, such as:

@supports (text-align: left) {
    /* rules */
}

I'm not sure if that's the correct behavior. What do you think?

[jeddy3]

Looking good! Two questions...

Do we want the rule to also flag the width and height size features (useful for alignment with the container type)?, i.e. allow this:

main {
  container-type: inline-size;

  & h2 {
    @container (inline-size > 40em) {
      /* .. */
    }
  }

While disallowing this:

main {
  container-type: inline-size;

  & h2 {
    @container (width > 40em) {
      /* .. */
    }
  }

And do we want to do anything special with overflow-x and overflow-y as their logical counterparts are only supported by Firefox?

[nzakas]

And do we want to do anything special with overflow-x and overflow-y as their logical counterparts are only supported by Firefox?

I think we can skip these for now. If/when other browsers implement the logical equivalent we can revisit.

Do we want the rule to also flag the width and height size features (useful for alignment with the container type)?,

These aren't properties, so I'm not sure it makes sense to bundle this check in this rule.

[azat-io]

This is looking good. I just realized that it will also error for @supports rules

You are right. We should not use linting in @supports directives. Stylelint's rules don't do that either.

I've updated the PR and added new tests.

[nzakas]

LGTM. Thanks for all of your work on this.

(The CI failure is unrelated to this change, so we're not going to worry about that.)