Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

new check: OrderedProperties #6311

Closed
thomassenger opened this issue Dec 23, 2018 · 5 comments

Comments

Projects
None yet
3 participants
@thomassenger
Copy link

commented Dec 23, 2018

I have written a new check OrderedPropertiesCheck. It checks property files if they are in correct sort order.
In the same way your development environment will do.
If you do merges and continuous builds, than you would like to check this automatically.

cat OrderedPropertiesCheckConfig.xml
<?xml version="1.0"?>
<!DOCTYPE module PUBLIC
        "-//Checkstyle//DTD Checkstyle Configuration 1.3//EN"
        "https://checkstyle.org/dtds/configuration_1_3.dtd">
<module name="Checker">
    <module name="OrderedPropertiesCheck"/>
</module>

Here some examples

Only the order of keys is important. Here the value are in correct order, but it is not relevant.

cat src/test/resources/com/puppycrawl/tools/checkstyle/checks/orderedproperties/InputOrderedProperties1OrderKey.properties
#only order of keys are important
key2=value1
key1=value2 - violation

Now correctly sorted:

cat src/test/resources/com/puppycrawl/tools/checkstyle/checks/orderedproperties/ExpectedOrderedProperties1OrderKey.properties
key1=value2
#only order of keys are important
key2=value1

Test the order of an empty value.

An empty value is not relevant. key11 was only added because I no not know how to "verify" without errors.

cat src/test/resources/com/puppycrawl/tools/checkstyle/checks/orderedproperties/InputOrderedProperties2EmptyValue.properties
key1=
key2=value2
key11=value1 - violation

Here we would like to use an Locale independent order mechanism, an binary order.

This is available with String.compareTo() method.
As reference you can use the ascii table.

  • The @ sign is on position 64 and comes first
  • The capital A is on 65 and the lowercase a is on position 97 on the ascii table
  • On line 4 you have an a umlaut. This belongs to the extended ascii table and comes at last position. This is the violation here.
  • Key and subkey are in correct order here, because only keys are relevant.
    Therefore on line 5 you have only "key" an nothing behind.
    On line 6 you have "key." The dot is on position 46 which is higher than nothing.
cat -b  src/test/resources/com/puppycrawl/tools/checkstyle/checks/orderedproperties/InputOrderedProperties3BinaryOrder.properties
     1	@=64
     2	A=65
     3	a=97
     4	ä=132 - violation
     5	key=107 than nothing
     6	key.sub=k is 107 and dot is 46

Number before text

cat src/test/resources/com/puppycrawl/tools/checkstyle/checks/orderedproperties/InputOrderedProperties4Numbers.properties
hello=Hello
# key cancel should be before hello
cancel=Cancel - violation
3=3 - violation
22=22 - violation
111=111 - violation
01=01 - violation

Now correctly sorted:

cat src/test/resources/com/puppycrawl/tools/checkstyle/checks/orderedproperties/ExpectedOrderedProperties4Numbers.properties
01=01
111=111
22=22
3=3
# key cancel should be before hello
cancel=Cancel
hello=Hello

Implementation

The implementation can be found here:
https://github.com/thomassenger/checkstyle/tree/master/src/main/java/com/puppycrawl/tools/checkstyle/checks/OrderedPropertiesCheck.java

thomassenger pushed a commit to thomassenger/checkstyle that referenced this issue Dec 23, 2018

@romani

This comment has been minimized.

Copy link
Member

commented Dec 24, 2018

Corresponding Inspecton is named AlphaUnsortedPropertiesFile- https://github.com/checkstyle/checkstyle/blob/master/config/intellij-idea-inspections.xml#L44
image

so it give us hint that sorting are different, example - http://www.alliancegroup.co.uk/windows7-explorer-sort-order.htm
Please define what type of ordering you are implementing.
We some historical reason we use "Order" noun for this https://checkstyle.org/checks.html
So name should be smth like "OrderedProperties", similar to https://checkstyle.org/config_misc.html#UniqueProperties.

thomassenger pushed a commit to thomassenger/checkstyle that referenced this issue Dec 26, 2018

thomassenger pushed a commit to thomassenger/checkstyle that referenced this issue Dec 29, 2018

@romani romani changed the title a new check SortedPropertiesCheck. new check: OrderedProperties Dec 30, 2018

thomassenger pushed a commit to thomassenger/checkstyle that referenced this issue Dec 30, 2018

thomassenger pushed a commit to thomassenger/checkstyle that referenced this issue Dec 30, 2018

@checkstyle checkstyle deleted a comment from thomassenger Dec 30, 2018

@checkstyle checkstyle deleted a comment from thomassenger Dec 30, 2018

@checkstyle checkstyle deleted a comment from thomassenger Dec 30, 2018

@checkstyle checkstyle deleted a comment from thomassenger Dec 30, 2018

@romani

This comment has been minimized.

Copy link
Member

commented Dec 30, 2018

@rnveach, please review, looks ok for me.

@rnveach

This comment has been minimized.

Copy link
Member

commented Dec 30, 2018

Minor details: I see no mention that comments and empty lines will be ignored in sorting process or an example of a multi-line property value. They should be in tests/xdoc when the PR is started.

Issue looks good to me. I will mark approve.

@romani

This comment has been minimized.

Copy link
Member

commented Dec 30, 2018

@thomassenger , now send PR with Check, and we will guide you on what is required to be done. Advise, just fix what CI do not like in your code. See you in PR

thomassenger pushed a commit to thomassenger/checkstyle that referenced this issue Jan 3, 2019

thomassenger pushed a commit to thomassenger/checkstyle that referenced this issue Jan 6, 2019

Thomas Senger
Issue checkstyle#6311: SortedPropertiesCheck (machine) translate reso…
…urce bundle, key:properties.notSorted.property

thomassenger pushed a commit to thomassenger/checkstyle that referenced this issue Jan 6, 2019

thomassenger pushed a commit to thomassenger/checkstyle that referenced this issue Jan 6, 2019

thomassenger pushed a commit to thomassenger/checkstyle that referenced this issue Jan 6, 2019

thomassenger pushed a commit to thomassenger/checkstyle that referenced this issue Jan 7, 2019

thomassenger pushed a commit to thomassenger/checkstyle that referenced this issue Jan 9, 2019

thomassenger pushed a commit to thomassenger/checkstyle that referenced this issue Jan 9, 2019

thomassenger pushed a commit to thomassenger/checkstyle that referenced this issue Jan 9, 2019

thomassenger pushed a commit to thomassenger/checkstyle that referenced this issue Jan 9, 2019

thomassenger pushed a commit to thomassenger/checkstyle that referenced this issue Jan 9, 2019

thomassenger pushed a commit to thomassenger/checkstyle that referenced this issue Jan 13, 2019

thomassenger pushed a commit to thomassenger/checkstyle that referenced this issue Jan 13, 2019

thomassenger pushed a commit to thomassenger/checkstyle that referenced this issue Apr 25, 2019

thomassenger pushed a commit to thomassenger/checkstyle that referenced this issue Apr 25, 2019

thomassenger pushed a commit to thomassenger/checkstyle that referenced this issue Apr 29, 2019

thomassenger pushed a commit to thomassenger/checkstyle that referenced this issue Apr 29, 2019

thomassenger pushed a commit to thomassenger/checkstyle that referenced this issue May 6, 2019

thomassenger pushed a commit to thomassenger/checkstyle that referenced this issue May 21, 2019

thomassenger pushed a commit to thomassenger/checkstyle that referenced this issue May 23, 2019

thomassenger pushed a commit to thomassenger/checkstyle that referenced this issue May 24, 2019

thomassenger pushed a commit to thomassenger/checkstyle that referenced this issue May 24, 2019

thomassenger pushed a commit to thomassenger/checkstyle that referenced this issue May 26, 2019

thomassenger pushed a commit to thomassenger/checkstyle that referenced this issue May 26, 2019

thomassenger pushed a commit to thomassenger/checkstyle that referenced this issue Jun 6, 2019

thomassenger pushed a commit to thomassenger/checkstyle that referenced this issue Jun 6, 2019

Thomas Senger
Issue checkstyle#6311: OrderedPropertiesCheck improve documentation, …
…fix failed XdocsPagesTest OutOfBoundsException

thomassenger pushed a commit to thomassenger/checkstyle that referenced this issue Jun 10, 2019

thomassenger pushed a commit to thomassenger/checkstyle that referenced this issue Jun 11, 2019

thomassenger pushed a commit to thomassenger/checkstyle that referenced this issue Jun 12, 2019

thomassenger pushed a commit to thomassenger/checkstyle that referenced this issue Jun 12, 2019

thomassenger pushed a commit to thomassenger/checkstyle that referenced this issue Jun 13, 2019

thomassenger pushed a commit to thomassenger/checkstyle that referenced this issue Jun 13, 2019

thomassenger pushed a commit to thomassenger/checkstyle that referenced this issue Jun 14, 2019

thomassenger pushed a commit to thomassenger/checkstyle that referenced this issue Jun 14, 2019

thomassenger pushed a commit to thomassenger/checkstyle that referenced this issue Jun 15, 2019

thomassenger pushed a commit to thomassenger/checkstyle that referenced this issue Jun 18, 2019

thomassenger pushed a commit to thomassenger/checkstyle that referenced this issue Jun 18, 2019

thomassenger pushed a commit to thomassenger/checkstyle that referenced this issue Jun 18, 2019

romani added a commit that referenced this issue Jun 21, 2019

@romani romani added this to the 8.22 milestone Jun 21, 2019

@romani

This comment has been minimized.

Copy link
Member

commented Jun 21, 2019

Fix is merged

@romani romani closed this Jun 21, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.