Investigate if Mention Bot can be smarter #6531
Assignees
Labels
archived due to age
This issue has been archived; please open a new issue for any further discussion
chore
This change is not user-facing
evaluating
The team will evaluate this issue to decide whether it meets the criteria for inclusion
infrastructure
Relates to the tools used in the ESLint development process
Comments
platinumazure
added
infrastructure
|
I had vaguely remembered that it is possible to add a configuration file to a consuming repository. The documentation for that can be found here. I'll submit a concrete PR this weekend. |
|
After further digging, I don't think it's possible to prioritize reviewers, only flat-out whitelist with no fallback. I assume we don't necessarily want that, but if we think it could help prioritize PRs for the TSC members (based on their own familiarity), I could write a PR for that if we want to try it. I can also reach out to the repo maintainers to see if the original proposal is an enhancement they would be willing to implement, if desired. My assumption at this point is that this isn't a high priority, so I'm going to leave this open for a bit and then close (unless a TSC member beats me to that or unless a discussion sprouts up). Thanks! |
|
Closing this issue as it's not a high priority. If anyone is interested in looking at this again, we can reopen. |
not-an-aardvark
added a commit
that referenced
this issue
Nov 18, 2016
Fixes #1801, fixes #3737, fixes #3845, fixes #6007, fixes #6571, fixes #6670, fixes #6813, fixes #7242, fixes #7274, fixes #7320, fixes #7420, fixes #7522, fixes #7616) The existing implementation of `indent` had a lot of bugs (see above list). It worked by detecting a node type (e.g. `ObjectExpression`), and then ensuring that the indentation around the object satisfies certain constraints (e.g. the properties of the `ObjectExpression` are offset by 4 spaces from the opening bracket). This approach had a number of disadvantages: - Since it only checked indentation according to an explicit list of patterns, there were a lot of cases where it accidentally didn't check the indentation at all. For example, there was no check for the indentation of a closing `)` in a `CallExpression`, so the rule just silently ignored incorrect indentation in these cases. (#7522) - there were a lot of nodes where indentation wasn't checked at all. For example, it didn't check indentation for ternary expressions (#7420) or destructuring assignments (#6813). - Since it could only check indent patterns on nodes, it couldn't check the indentation of comments (#3845, #6571) or optional tokens such as parentheses around an expression (#7522) This commit rewrites the `indent` rule. The new strategy is based on tokens rather than nodes: 1. Create a hashmap (`desiredOffsets`). The keys are all the tokens and comments in the file, and the values are objects containing information for a specific offset, measured in indent levels, from a either a specific token or the first column. For example, an element in an array will have `{offset: 1, from: openingCurly}` to indicate that it is offset by one indentation level from the opening curly brace. All the offsets are initialized to 0 at the start. 1. As the AST is traversed, modify the offsets of tokens accordingly. For example, when entering a `BlockStatement`, offset all of the tokens in the `BlockStatement` by 1 from the opening curly brace of the `BlockStatement`. 1. After traversing the AST, calculate the expected indentation levels of every token in the file (according to the `desiredOffsets` map). 1. For each token, compare the expected indentation to the actual indentation in the file, and report the token if the two values are not equal. This has the following advantages: - It is guaranteed to check the indentation of every single token in the file, with the exception of some tokens that are explicitly ignored*. This ensures that no tokens end up unexpectedly being ignored. - Since tokens/comments are used instead of nodes, there are no unchecked "stray tokens". - All nodes are evaluated in a context-free manner. In other words, each node only has to set an offset for its own children, without worrying about what how much indentation the node itself has or what the node's parents are. - The rule ends up with an expected indentation map for the entire file at once, and so it can fix the entire file in one pass. (The previous implementation often required multiple passes. For example, if a node was misaligned with its parent in the previous implementation, the node would get fixed, even if the node's position was actually correct and the parent was off.) *There are a few cases where the new implementation explicitly ignores lines. I decided to do this because there is a huge amount of inconsistency in what people seem to prefer for these cases. If this gets released in a major version, we might want to stop ignoring these cases so that the indentation of all lines is checked. One such case is: ```js ({ foo: bar }); // versus ({ foo: bar }); ``` Comments are treated a bit differently from tokens in that they can have several different indentations. This is because it can be difficult to tell what the comment is referring to. For example: ```js if (foo) { doSomething(); // comment about the doSomething() call } else if (bar.baz()) { doSomethingElse(); } // versus if (foo) { doSomething(); // comment about the bar.baz() call } else if (bar.baz()) { doSomethingElse(); } ``` Specifically, a comment is allowed to have one of three indentations: 1. The same indentation as the token right before it 1. The same indentation as the token right after it 1. The computed indentation for the comment itself --- As mentioned above, the new implementation checks a lot of cases that the old implementation did not check. Based on past experience with fixing bugs in the `indent` rule, I anticipate that this will break quite a lot of builds. We might want to consider holding this off until we release a major version.
not-an-aardvark
added a commit
that referenced
this issue
Dec 12, 2016
Fixes #1801, fixes #3737, fixes #3845, fixes #6007, fixes #6571, fixes #6670, fixes #6813, fixes #7242, fixes #7274, fixes #7320, fixes #7420, fixes #7522, fixes #7616) The existing implementation of `indent` had a lot of bugs (see above list). It worked by detecting a node type (e.g. `ObjectExpression`), and then ensuring that the indentation around the object satisfies certain constraints (e.g. the properties of the `ObjectExpression` are offset by 4 spaces from the opening bracket). This approach had a number of disadvantages: - Since it only checked indentation according to an explicit list of patterns, there were a lot of cases where it accidentally didn't check the indentation at all. For example, there was no check for the indentation of a closing `)` in a `CallExpression`, so the rule just silently ignored incorrect indentation in these cases. (#7522) - there were a lot of nodes where indentation wasn't checked at all. For example, it didn't check indentation for ternary expressions (#7420) or destructuring assignments (#6813). - Since it could only check indent patterns on nodes, it couldn't check the indentation of comments (#3845, #6571) or optional tokens such as parentheses around an expression (#7522) This commit rewrites the `indent` rule. The new strategy is based on tokens rather than nodes: 1. Create a hashmap (`desiredOffsets`). The keys are all the tokens and comments in the file, and the values are objects containing information for a specific offset, measured in indent levels, from a either a specific token or the first column. For example, an element in an array will have `{offset: 1, from: openingCurly}` to indicate that it is offset by one indentation level from the opening curly brace. All the offsets are initialized to 0 at the start. 1. As the AST is traversed, modify the offsets of tokens accordingly. For example, when entering a `BlockStatement`, offset all of the tokens in the `BlockStatement` by 1 from the opening curly brace of the `BlockStatement`. 1. After traversing the AST, calculate the expected indentation levels of every token in the file (according to the `desiredOffsets` map). 1. For each token, compare the expected indentation to the actual indentation in the file, and report the token if the two values are not equal. This has the following advantages: - It is guaranteed to check the indentation of every single token in the file, with the exception of some tokens that are explicitly ignored*. This ensures that no tokens end up unexpectedly being ignored. - Since tokens/comments are used instead of nodes, there are no unchecked "stray tokens". - All nodes are evaluated in a context-free manner. In other words, each node only has to set an offset for its own children, without worrying about what how much indentation the node itself has or what the node's parents are. - The rule ends up with an expected indentation map for the entire file at once, and so it can fix the entire file in one pass. (The previous implementation often required multiple passes. For example, if a node was misaligned with its parent in the previous implementation, the node would get fixed, even if the node's position was actually correct and the parent was off.) *There are a few cases where the new implementation explicitly ignores lines. I decided to do this because there is a huge amount of inconsistency in what people seem to prefer for these cases. If this gets released in a major version, we might want to stop ignoring these cases so that the indentation of all lines is checked. One such case is: ```js ({ foo: bar }); // versus ({ foo: bar }); ``` Comments are treated a bit differently from tokens in that they can have several different indentations. This is because it can be difficult to tell what the comment is referring to. For example: ```js if (foo) { doSomething(); // comment about the doSomething() call } else if (bar.baz()) { doSomethingElse(); } // versus if (foo) { doSomething(); // comment about the bar.baz() call } else if (bar.baz()) { doSomethingElse(); } ``` Specifically, a comment is allowed to have one of three indentations: 1. The same indentation as the token right before it 1. The same indentation as the token right after it 1. The computed indentation for the comment itself --- As mentioned above, the new implementation checks a lot of cases that the old implementation did not check. Based on past experience with fixing bugs in the `indent` rule, I anticipate that this will break quite a lot of builds. We might want to consider holding this off until we release a major version.
not-an-aardvark
added a commit
that referenced
this issue
Dec 12, 2016
Fixes #1801, fixes #3737, fixes #3845, fixes #6007, fixes #6571, fixes #6670, fixes #6813, fixes #7242, fixes #7274, fixes #7320, fixes #7420, fixes #7522, fixes #7616) The existing implementation of `indent` had a lot of bugs (see above list). It worked by detecting a node type (e.g. `ObjectExpression`), and then ensuring that the indentation around the object satisfies certain constraints (e.g. the properties of the `ObjectExpression` are offset by 4 spaces from the opening bracket). This approach had a number of disadvantages: - Since it only checked indentation according to an explicit list of patterns, there were a lot of cases where it accidentally didn't check the indentation at all. For example, there was no check for the indentation of a closing `)` in a `CallExpression`, so the rule just silently ignored incorrect indentation in these cases. (#7522) - there were a lot of nodes where indentation wasn't checked at all. For example, it didn't check indentation for ternary expressions (#7420) or destructuring assignments (#6813). - Since it could only check indent patterns on nodes, it couldn't check the indentation of comments (#3845, #6571) or optional tokens such as parentheses around an expression (#7522) This commit rewrites the `indent` rule. The new strategy is based on tokens rather than nodes: 1. Create a hashmap (`desiredOffsets`). The keys are all the tokens and comments in the file, and the values are objects containing information for a specific offset, measured in indent levels, from a either a specific token or the first column. For example, an element in an array will have `{offset: 1, from: openingCurly}` to indicate that it is offset by one indentation level from the opening curly brace. All the offsets are initialized to 0 at the start. 1. As the AST is traversed, modify the offsets of tokens accordingly. For example, when entering a `BlockStatement`, offset all of the tokens in the `BlockStatement` by 1 from the opening curly brace of the `BlockStatement`. 1. After traversing the AST, calculate the expected indentation levels of every token in the file (according to the `desiredOffsets` map). 1. For each token, compare the expected indentation to the actual indentation in the file, and report the token if the two values are not equal. This has the following advantages: - It is guaranteed to check the indentation of every single token in the file, with the exception of some tokens that are explicitly ignored*. This ensures that no tokens end up unexpectedly being ignored. - Since tokens/comments are used instead of nodes, there are no unchecked "stray tokens". - All nodes are evaluated in a context-free manner. In other words, each node only has to set an offset for its own children, without worrying about what how much indentation the node itself has or what the node's parents are. - The rule ends up with an expected indentation map for the entire file at once, and so it can fix the entire file in one pass. (The previous implementation often required multiple passes. For example, if a node was misaligned with its parent in the previous implementation, the node would get fixed, even if the node's position was actually correct and the parent was off.) *There are a few cases where the new implementation explicitly ignores lines. I decided to do this because there is a huge amount of inconsistency in what people seem to prefer for these cases. If this gets released in a major version, we might want to stop ignoring these cases so that the indentation of all lines is checked. One such case is: ```js ({ foo: bar }); // versus ({ foo: bar }); ``` Comments are treated a bit differently from tokens in that they can have several different indentations. This is because it can be difficult to tell what the comment is referring to. For example: ```js if (foo) { doSomething(); // comment about the doSomething() call } else if (bar.baz()) { doSomethingElse(); } // versus if (foo) { doSomething(); // comment about the bar.baz() call } else if (bar.baz()) { doSomethingElse(); } ``` Specifically, a comment is allowed to have one of three indentations: 1. The same indentation as the token right before it 1. The same indentation as the token right after it 1. The computed indentation for the comment itself --- As mentioned above, the new implementation checks a lot of cases that the old implementation did not check. Based on past experience with fixing bugs in the `indent` rule, I anticipate that this will break quite a lot of builds. We might want to consider holding this off until we release a major version.
not-an-aardvark
added a commit
that referenced
this issue
Dec 14, 2016
Fixes #1801, fixes #3737, fixes #3845, fixes #6007, fixes #6571, fixes #6670, fixes #6813, fixes #7242, fixes #7274, fixes #7320, fixes #7420, fixes #7522, fixes #7616) The existing implementation of `indent` had a lot of bugs (see above list). It worked by detecting a node type (e.g. `ObjectExpression`), and then ensuring that the indentation around the object satisfies certain constraints (e.g. the properties of the `ObjectExpression` are offset by 4 spaces from the opening bracket). This approach had a number of disadvantages: - Since it only checked indentation according to an explicit list of patterns, there were a lot of cases where it accidentally didn't check the indentation at all. For example, there was no check for the indentation of a closing `)` in a `CallExpression`, so the rule just silently ignored incorrect indentation in these cases. (#7522) - there were a lot of nodes where indentation wasn't checked at all. For example, it didn't check indentation for ternary expressions (#7420) or destructuring assignments (#6813). - Since it could only check indent patterns on nodes, it couldn't check the indentation of comments (#3845, #6571) or optional tokens such as parentheses around an expression (#7522) This commit rewrites the `indent` rule. The new strategy is based on tokens rather than nodes: 1. Create a hashmap (`desiredOffsets`). The keys are all the tokens and comments in the file, and the values are objects containing information for a specific offset, measured in indent levels, from a either a specific token or the first column. For example, an element in an array will have `{offset: 1, from: openingCurly}` to indicate that it is offset by one indentation level from the opening curly brace. All the offsets are initialized to 0 at the start. 1. As the AST is traversed, modify the offsets of tokens accordingly. For example, when entering a `BlockStatement`, offset all of the tokens in the `BlockStatement` by 1 from the opening curly brace of the `BlockStatement`. 1. After traversing the AST, calculate the expected indentation levels of every token in the file (according to the `desiredOffsets` map). 1. For each token, compare the expected indentation to the actual indentation in the file, and report the token if the two values are not equal. This has the following advantages: - It is guaranteed to check the indentation of every single token in the file, with the exception of some tokens that are explicitly ignored*. This ensures that no tokens end up unexpectedly being ignored. - Since tokens/comments are used instead of nodes, there are no unchecked "stray tokens". - All nodes are evaluated in a context-free manner. In other words, each node only has to set an offset for its own children, without worrying about what how much indentation the node itself has or what the node's parents are. - The rule ends up with an expected indentation map for the entire file at once, and so it can fix the entire file in one pass. (The previous implementation often required multiple passes. For example, if a node was misaligned with its parent in the previous implementation, the node would get fixed, even if the node's position was actually correct and the parent was off.) *There are a few cases where the new implementation explicitly ignores lines. I decided to do this because there is a huge amount of inconsistency in what people seem to prefer for these cases. If this gets released in a major version, we might want to stop ignoring these cases so that the indentation of all lines is checked. One such case is: ```js ({ foo: bar }); // versus ({ foo: bar }); ``` Comments are treated a bit differently from tokens in that they can have several different indentations. This is because it can be difficult to tell what the comment is referring to. For example: ```js if (foo) { doSomething(); // comment about the doSomething() call } else if (bar.baz()) { doSomethingElse(); } // versus if (foo) { doSomething(); // comment about the bar.baz() call } else if (bar.baz()) { doSomethingElse(); } ``` Specifically, a comment is allowed to have one of three indentations: 1. The same indentation as the token right before it 1. The same indentation as the token right after it 1. The computed indentation for the comment itself --- As mentioned above, the new implementation checks a lot of cases that the old implementation did not check. Based on past experience with fixing bugs in the `indent` rule, I anticipate that this will break quite a lot of builds. We might want to consider holding this off until we release a major version.
not-an-aardvark
added a commit
that referenced
this issue
Dec 14, 2016
Fixes #1801, fixes #3737, fixes #3845, fixes #6007, fixes #6571, fixes #6670, fixes #6813, fixes #7242, fixes #7274, fixes #7320, fixes #7420, fixes #7522, fixes #7616) The existing implementation of `indent` had a lot of bugs (see above list). It worked by detecting a node type (e.g. `ObjectExpression`), and then ensuring that the indentation around the object satisfies certain constraints (e.g. the properties of the `ObjectExpression` are offset by 4 spaces from the opening bracket). This approach had a number of disadvantages: - Since it only checked indentation according to an explicit list of patterns, there were a lot of cases where it accidentally didn't check the indentation at all. For example, there was no check for the indentation of a closing `)` in a `CallExpression`, so the rule just silently ignored incorrect indentation in these cases. (#7522) - there were a lot of nodes where indentation wasn't checked at all. For example, it didn't check indentation for ternary expressions (#7420) or destructuring assignments (#6813). - Since it could only check indent patterns on nodes, it couldn't check the indentation of comments (#3845, #6571) or optional tokens such as parentheses around an expression (#7522) This commit rewrites the `indent` rule. The new strategy is based on tokens rather than nodes: 1. Create a hashmap (`desiredOffsets`). The keys are all the tokens and comments in the file, and the values are objects containing information for a specific offset, measured in indent levels, from a either a specific token or the first column. For example, an element in an array will have `{offset: 1, from: openingCurly}` to indicate that it is offset by one indentation level from the opening curly brace. All the offsets are initialized to 0 at the start. 1. As the AST is traversed, modify the offsets of tokens accordingly. For example, when entering a `BlockStatement`, offset all of the tokens in the `BlockStatement` by 1 from the opening curly brace of the `BlockStatement`. 1. After traversing the AST, calculate the expected indentation levels of every token in the file (according to the `desiredOffsets` map). 1. For each token, compare the expected indentation to the actual indentation in the file, and report the token if the two values are not equal. This has the following advantages: - It is guaranteed to check the indentation of every single token in the file, with the exception of some tokens that are explicitly ignored*. This ensures that no tokens end up unexpectedly being ignored. - Since tokens/comments are used instead of nodes, there are no unchecked "stray tokens". - All nodes are evaluated in a context-free manner. In other words, each node only has to set an offset for its own children, without worrying about what how much indentation the node itself has or what the node's parents are. - The rule ends up with an expected indentation map for the entire file at once, and so it can fix the entire file in one pass. (The previous implementation often required multiple passes. For example, if a node was misaligned with its parent in the previous implementation, the node would get fixed, even if the node's position was actually correct and the parent was off.) *There are a few cases where the new implementation explicitly ignores lines. I decided to do this because there is a huge amount of inconsistency in what people seem to prefer for these cases. If this gets released in a major version, we might want to stop ignoring these cases so that the indentation of all lines is checked. One such case is: ```js ({ foo: bar }); // versus ({ foo: bar }); ``` Comments are treated a bit differently from tokens in that they can have several different indentations. This is because it can be difficult to tell what the comment is referring to. For example: ```js if (foo) { doSomething(); // comment about the doSomething() call } else if (bar.baz()) { doSomethingElse(); } // versus if (foo) { doSomething(); // comment about the bar.baz() call } else if (bar.baz()) { doSomethingElse(); } ``` Specifically, a comment is allowed to have one of three indentations: 1. The same indentation as the token right before it 1. The same indentation as the token right after it 1. The computed indentation for the comment itself --- As mentioned above, the new implementation checks a lot of cases that the old implementation did not check. Based on past experience with fixing bugs in the `indent` rule, I anticipate that this will break quite a lot of builds. We might want to consider holding this off until we release a major version.
not-an-aardvark
added a commit
that referenced
this issue
Jan 2, 2017
Fixes #1801, fixes #3737, fixes #3845, fixes #6007, fixes #6571, fixes #6670, fixes #6813, fixes #7242, fixes #7274, fixes #7320, fixes #7420, fixes #7522, fixes #7616) The existing implementation of `indent` had a lot of bugs (see above list). It worked by detecting a node type (e.g. `ObjectExpression`), and then ensuring that the indentation around the object satisfies certain constraints (e.g. the properties of the `ObjectExpression` are offset by 4 spaces from the opening bracket). This approach had a number of disadvantages: - Since it only checked indentation according to an explicit list of patterns, there were a lot of cases where it accidentally didn't check the indentation at all. For example, there was no check for the indentation of a closing `)` in a `CallExpression`, so the rule just silently ignored incorrect indentation in these cases. (#7522) - there were a lot of nodes where indentation wasn't checked at all. For example, it didn't check indentation for ternary expressions (#7420) or destructuring assignments (#6813). - Since it could only check indent patterns on nodes, it couldn't check the indentation of comments (#3845, #6571) or optional tokens such as parentheses around an expression (#7522) This commit rewrites the `indent` rule. The new strategy is based on tokens rather than nodes: 1. Create a hashmap (`desiredOffsets`). The keys are all the tokens and comments in the file, and the values are objects containing information for a specific offset, measured in indent levels, from a either a specific token or the first column. For example, an element in an array will have `{offset: 1, from: openingCurly}` to indicate that it is offset by one indentation level from the opening curly brace. All the offsets are initialized to 0 at the start. 1. As the AST is traversed, modify the offsets of tokens accordingly. For example, when entering a `BlockStatement`, offset all of the tokens in the `BlockStatement` by 1 from the opening curly brace of the `BlockStatement`. 1. After traversing the AST, calculate the expected indentation levels of every token in the file (according to the `desiredOffsets` map). 1. For each token, compare the expected indentation to the actual indentation in the file, and report the token if the two values are not equal. This has the following advantages: - It is guaranteed to check the indentation of every single token in the file, with the exception of some tokens that are explicitly ignored*. This ensures that no tokens end up unexpectedly being ignored. - Since tokens/comments are used instead of nodes, there are no unchecked "stray tokens". - All nodes are evaluated in a context-free manner. In other words, each node only has to set an offset for its own children, without worrying about what how much indentation the node itself has or what the node's parents are. - The rule ends up with an expected indentation map for the entire file at once, and so it can fix the entire file in one pass. (The previous implementation often required multiple passes. For example, if a node was misaligned with its parent in the previous implementation, the node would get fixed, even if the node's position was actually correct and the parent was off.) *There are a few cases where the new implementation explicitly ignores lines. I decided to do this because there is a huge amount of inconsistency in what people seem to prefer for these cases. If this gets released in a major version, we might want to stop ignoring these cases so that the indentation of all lines is checked. One such case is: ```js ({ foo: bar }); // versus ({ foo: bar }); ``` Comments are treated a bit differently from tokens in that they can have several different indentations. This is because it can be difficult to tell what the comment is referring to. For example: ```js if (foo) { doSomething(); // comment about the doSomething() call } else if (bar.baz()) { doSomethingElse(); } // versus if (foo) { doSomething(); // comment about the bar.baz() call } else if (bar.baz()) { doSomethingElse(); } ``` Specifically, a comment is allowed to have one of three indentations: 1. The same indentation as the token right before it 1. The same indentation as the token right after it 1. The computed indentation for the comment itself --- As mentioned above, the new implementation checks a lot of cases that the old implementation did not check. Based on past experience with fixing bugs in the `indent` rule, I anticipate that this will break quite a lot of builds. We might want to consider holding this off until we release a major version.
not-an-aardvark
added a commit
that referenced
this issue
Jan 10, 2017
Fixes #1801, fixes #3737, fixes #3845, fixes #6007, fixes #6571, fixes #6670, fixes #6813, fixes #7242, fixes #7274, fixes #7320, fixes #7420, fixes #7522, fixes #7616) The existing implementation of `indent` had a lot of bugs (see above list). It worked by detecting a node type (e.g. `ObjectExpression`), and then ensuring that the indentation around the object satisfies certain constraints (e.g. the properties of the `ObjectExpression` are offset by 4 spaces from the opening bracket). This approach had a number of disadvantages: - Since it only checked indentation according to an explicit list of patterns, there were a lot of cases where it accidentally didn't check the indentation at all. For example, there was no check for the indentation of a closing `)` in a `CallExpression`, so the rule just silently ignored incorrect indentation in these cases. (#7522) - there were a lot of nodes where indentation wasn't checked at all. For example, it didn't check indentation for ternary expressions (#7420) or destructuring assignments (#6813). - Since it could only check indent patterns on nodes, it couldn't check the indentation of comments (#3845, #6571) or optional tokens such as parentheses around an expression (#7522) This commit rewrites the `indent` rule. The new strategy is based on tokens rather than nodes: 1. Create a hashmap (`desiredOffsets`). The keys are all the tokens and comments in the file, and the values are objects containing information for a specific offset, measured in indent levels, from a either a specific token or the first column. For example, an element in an array will have `{offset: 1, from: openingCurly}` to indicate that it is offset by one indentation level from the opening curly brace. All the offsets are initialized to 0 at the start. 1. As the AST is traversed, modify the offsets of tokens accordingly. For example, when entering a `BlockStatement`, offset all of the tokens in the `BlockStatement` by 1 from the opening curly brace of the `BlockStatement`. 1. After traversing the AST, calculate the expected indentation levels of every token in the file (according to the `desiredOffsets` map). 1. For each token, compare the expected indentation to the actual indentation in the file, and report the token if the two values are not equal. This has the following advantages: - It is guaranteed to check the indentation of every single token in the file, with the exception of some tokens that are explicitly ignored*. This ensures that no tokens end up unexpectedly being ignored. - Since tokens/comments are used instead of nodes, there are no unchecked "stray tokens". - All nodes are evaluated in a context-free manner. In other words, each node only has to set an offset for its own children, without worrying about what how much indentation the node itself has or what the node's parents are. - The rule ends up with an expected indentation map for the entire file at once, and so it can fix the entire file in one pass. (The previous implementation often required multiple passes. For example, if a node was misaligned with its parent in the previous implementation, the node would get fixed, even if the node's position was actually correct and the parent was off.) *There are a few cases where the new implementation explicitly ignores lines. I decided to do this because there is a huge amount of inconsistency in what people seem to prefer for these cases. If this gets released in a major version, we might want to stop ignoring these cases so that the indentation of all lines is checked. One such case is: ```js ({ foo: bar }); // versus ({ foo: bar }); ``` Comments are treated a bit differently from tokens in that they can have several different indentations. This is because it can be difficult to tell what the comment is referring to. For example: ```js if (foo) { doSomething(); // comment about the doSomething() call } else if (bar.baz()) { doSomethingElse(); } // versus if (foo) { doSomething(); // comment about the bar.baz() call } else if (bar.baz()) { doSomethingElse(); } ``` Specifically, a comment is allowed to have one of three indentations: 1. The same indentation as the token right before it 1. The same indentation as the token right after it 1. The computed indentation for the comment itself --- As mentioned above, the new implementation checks a lot of cases that the old implementation did not check. Based on past experience with fixing bugs in the `indent` rule, I anticipate that this will break quite a lot of builds. We might want to consider holding this off until we release a major version.
not-an-aardvark
added a commit
that referenced
this issue
Jan 14, 2017
Fixes #1801, fixes #3737, fixes #3845, fixes #6007, fixes #6571, fixes #6670, fixes #6813, fixes #7242, fixes #7274, fixes #7320, fixes #7420, fixes #7522, fixes #7616) The existing implementation of `indent` had a lot of bugs (see above list). It worked by detecting a node type (e.g. `ObjectExpression`), and then ensuring that the indentation around the object satisfies certain constraints (e.g. the properties of the `ObjectExpression` are offset by 4 spaces from the opening bracket). This approach had a number of disadvantages: - Since it only checked indentation according to an explicit list of patterns, there were a lot of cases where it accidentally didn't check the indentation at all. For example, there was no check for the indentation of a closing `)` in a `CallExpression`, so the rule just silently ignored incorrect indentation in these cases. (#7522) - there were a lot of nodes where indentation wasn't checked at all. For example, it didn't check indentation for ternary expressions (#7420) or destructuring assignments (#6813). - Since it could only check indent patterns on nodes, it couldn't check the indentation of comments (#3845, #6571) or optional tokens such as parentheses around an expression (#7522) This commit rewrites the `indent` rule. The new strategy is based on tokens rather than nodes: 1. Create a hashmap (`desiredOffsets`). The keys are all the tokens and comments in the file, and the values are objects containing information for a specific offset, measured in indent levels, from a either a specific token or the first column. For example, an element in an array will have `{offset: 1, from: openingCurly}` to indicate that it is offset by one indentation level from the opening curly brace. All the offsets are initialized to 0 at the start. 1. As the AST is traversed, modify the offsets of tokens accordingly. For example, when entering a `BlockStatement`, offset all of the tokens in the `BlockStatement` by 1 from the opening curly brace of the `BlockStatement`. 1. After traversing the AST, calculate the expected indentation levels of every token in the file (according to the `desiredOffsets` map). 1. For each token, compare the expected indentation to the actual indentation in the file, and report the token if the two values are not equal. This has the following advantages: - It is guaranteed to check the indentation of every single token in the file, with the exception of some tokens that are explicitly ignored*. This ensures that no tokens end up unexpectedly being ignored. - Since tokens/comments are used instead of nodes, there are no unchecked "stray tokens". - All nodes are evaluated in a context-free manner. In other words, each node only has to set an offset for its own children, without worrying about what how much indentation the node itself has or what the node's parents are. - The rule ends up with an expected indentation map for the entire file at once, and so it can fix the entire file in one pass. (The previous implementation often required multiple passes. For example, if a node was misaligned with its parent in the previous implementation, the node would get fixed, even if the node's position was actually correct and the parent was off.) *There are a few cases where the new implementation explicitly ignores lines. I decided to do this because there is a huge amount of inconsistency in what people seem to prefer for these cases. If this gets released in a major version, we might want to stop ignoring these cases so that the indentation of all lines is checked. One such case is: ```js ({ foo: bar }); // versus ({ foo: bar }); ``` Comments are treated a bit differently from tokens in that they can have several different indentations. This is because it can be difficult to tell what the comment is referring to. For example: ```js if (foo) { doSomething(); // comment about the doSomething() call } else if (bar.baz()) { doSomethingElse(); } // versus if (foo) { doSomething(); // comment about the bar.baz() call } else if (bar.baz()) { doSomethingElse(); } ``` Specifically, a comment is allowed to have one of three indentations: 1. The same indentation as the token right before it 1. The same indentation as the token right after it 1. The computed indentation for the comment itself --- As mentioned above, the new implementation checks a lot of cases that the old implementation did not check. Based on past experience with fixing bugs in the `indent` rule, I anticipate that this will break quite a lot of builds. We might want to consider holding this off until we release a major version.
not-an-aardvark
added a commit
that referenced
this issue
Jan 28, 2017
Fixes #1801, fixes #3737, fixes #3845, fixes #6007, fixes #6571, fixes #6670, fixes #6813, fixes #7242, fixes #7274, fixes #7320, fixes #7420, fixes #7522, fixes #7616) The existing implementation of `indent` had a lot of bugs (see above list). It worked by detecting a node type (e.g. `ObjectExpression`), and then ensuring that the indentation around the object satisfies certain constraints (e.g. the properties of the `ObjectExpression` are offset by 4 spaces from the opening bracket). This approach had a number of disadvantages: - Since it only checked indentation according to an explicit list of patterns, there were a lot of cases where it accidentally didn't check the indentation at all. For example, there was no check for the indentation of a closing `)` in a `CallExpression`, so the rule just silently ignored incorrect indentation in these cases. (#7522) - there were a lot of nodes where indentation wasn't checked at all. For example, it didn't check indentation for ternary expressions (#7420) or destructuring assignments (#6813). - Since it could only check indent patterns on nodes, it couldn't check the indentation of comments (#3845, #6571) or optional tokens such as parentheses around an expression (#7522) This commit rewrites the `indent` rule. The new strategy is based on tokens rather than nodes: 1. Create a hashmap (`desiredOffsets`). The keys are all the tokens and comments in the file, and the values are objects containing information for a specific offset, measured in indent levels, from a either a specific token or the first column. For example, an element in an array will have `{offset: 1, from: openingCurly}` to indicate that it is offset by one indentation level from the opening curly brace. All the offsets are initialized to 0 at the start. 1. As the AST is traversed, modify the offsets of tokens accordingly. For example, when entering a `BlockStatement`, offset all of the tokens in the `BlockStatement` by 1 from the opening curly brace of the `BlockStatement`. 1. After traversing the AST, calculate the expected indentation levels of every token in the file (according to the `desiredOffsets` map). 1. For each token, compare the expected indentation to the actual indentation in the file, and report the token if the two values are not equal. This has the following advantages: - It is guaranteed to check the indentation of every single token in the file, with the exception of some tokens that are explicitly ignored*. This ensures that no tokens end up unexpectedly being ignored. - Since tokens/comments are used instead of nodes, there are no unchecked "stray tokens". - All nodes are evaluated in a context-free manner. In other words, each node only has to set an offset for its own children, without worrying about what how much indentation the node itself has or what the node's parents are. - The rule ends up with an expected indentation map for the entire file at once, and so it can fix the entire file in one pass. (The previous implementation often required multiple passes. For example, if a node was misaligned with its parent in the previous implementation, the node would get fixed, even if the node's position was actually correct and the parent was off.) *There are a few cases where the new implementation explicitly ignores lines. I decided to do this because there is a huge amount of inconsistency in what people seem to prefer for these cases. If this gets released in a major version, we might want to stop ignoring these cases so that the indentation of all lines is checked. One such case is: ```js ({ foo: bar }); // versus ({ foo: bar }); ``` Comments are treated a bit differently from tokens in that they can have several different indentations. This is because it can be difficult to tell what the comment is referring to. For example: ```js if (foo) { doSomething(); // comment about the doSomething() call } else if (bar.baz()) { doSomethingElse(); } // versus if (foo) { doSomething(); // comment about the bar.baz() call } else if (bar.baz()) { doSomethingElse(); } ``` Specifically, a comment is allowed to have one of three indentations: 1. The same indentation as the token right before it 1. The same indentation as the token right after it 1. The computed indentation for the comment itself --- As mentioned above, the new implementation checks a lot of cases that the old implementation did not check. Based on past experience with fixing bugs in the `indent` rule, I anticipate that this will break quite a lot of builds. We might want to consider holding this off until we release a major version.
not-an-aardvark
added a commit
that referenced
this issue
Feb 10, 2017
Fixes #1801, fixes #3737, fixes #3845, fixes #6007, fixes #6571, fixes #6670, fixes #6813, fixes #7242, fixes #7274, fixes #7320, fixes #7420, fixes #7522, fixes #7616) The existing implementation of `indent` had a lot of bugs (see above list). It worked by detecting a node type (e.g. `ObjectExpression`), and then ensuring that the indentation around the object satisfies certain constraints (e.g. the properties of the `ObjectExpression` are offset by 4 spaces from the opening bracket). This approach had a number of disadvantages: - Since it only checked indentation according to an explicit list of patterns, there were a lot of cases where it accidentally didn't check the indentation at all. For example, there was no check for the indentation of a closing `)` in a `CallExpression`, so the rule just silently ignored incorrect indentation in these cases. (#7522) - there were a lot of nodes where indentation wasn't checked at all. For example, it didn't check indentation for ternary expressions (#7420) or destructuring assignments (#6813). - Since it could only check indent patterns on nodes, it couldn't check the indentation of comments (#3845, #6571) or optional tokens such as parentheses around an expression (#7522) This commit rewrites the `indent` rule. The new strategy is based on tokens rather than nodes: 1. Create a hashmap (`desiredOffsets`). The keys are all the tokens and comments in the file, and the values are objects containing information for a specific offset, measured in indent levels, from a either a specific token or the first column. For example, an element in an array will have `{offset: 1, from: openingCurly}` to indicate that it is offset by one indentation level from the opening curly brace. All the offsets are initialized to 0 at the start. 1. As the AST is traversed, modify the offsets of tokens accordingly. For example, when entering a `BlockStatement`, offset all of the tokens in the `BlockStatement` by 1 from the opening curly brace of the `BlockStatement`. 1. After traversing the AST, calculate the expected indentation levels of every token in the file (according to the `desiredOffsets` map). 1. For each token, compare the expected indentation to the actual indentation in the file, and report the token if the two values are not equal. This has the following advantages: - It is guaranteed to check the indentation of every single token in the file, with the exception of some tokens that are explicitly ignored*. This ensures that no tokens end up unexpectedly being ignored. - Since tokens/comments are used instead of nodes, there are no unchecked "stray tokens". - All nodes are evaluated in a context-free manner. In other words, each node only has to set an offset for its own children, without worrying about what how much indentation the node itself has or what the node's parents are. - The rule ends up with an expected indentation map for the entire file at once, and so it can fix the entire file in one pass. (The previous implementation often required multiple passes. For example, if a node was misaligned with its parent in the previous implementation, the node would get fixed, even if the node's position was actually correct and the parent was off.) *There are a few cases where the new implementation explicitly ignores lines. I decided to do this because there is a huge amount of inconsistency in what people seem to prefer for these cases. If this gets released in a major version, we might want to stop ignoring these cases so that the indentation of all lines is checked. One such case is: ```js ({ foo: bar }); // versus ({ foo: bar }); ``` Comments are treated a bit differently from tokens in that they can have several different indentations. This is because it can be difficult to tell what the comment is referring to. For example: ```js if (foo) { doSomething(); // comment about the doSomething() call } else if (bar.baz()) { doSomethingElse(); } // versus if (foo) { doSomething(); // comment about the bar.baz() call } else if (bar.baz()) { doSomethingElse(); } ``` Specifically, a comment is allowed to have one of three indentations: 1. The same indentation as the token right before it 1. The same indentation as the token right after it 1. The computed indentation for the comment itself --- As mentioned above, the new implementation checks a lot of cases that the old implementation did not check. Based on past experience with fixing bugs in the `indent` rule, I anticipate that this will break quite a lot of builds. We might want to consider holding this off until we release a major version.
not-an-aardvark
added a commit
that referenced
this issue
Feb 13, 2017
Fixes #1801, fixes #3737, fixes #3845, fixes #6007, fixes #6571, fixes #6670, fixes #6813, fixes #7242, fixes #7274, fixes #7320, fixes #7420, fixes #7522, fixes #7616) The existing implementation of `indent` had a lot of bugs (see above list). It worked by detecting a node type (e.g. `ObjectExpression`), and then ensuring that the indentation around the object satisfies certain constraints (e.g. the properties of the `ObjectExpression` are offset by 4 spaces from the opening bracket). This approach had a number of disadvantages: - Since it only checked indentation according to an explicit list of patterns, there were a lot of cases where it accidentally didn't check the indentation at all. For example, there was no check for the indentation of a closing `)` in a `CallExpression`, so the rule just silently ignored incorrect indentation in these cases. (#7522) - there were a lot of nodes where indentation wasn't checked at all. For example, it didn't check indentation for ternary expressions (#7420) or destructuring assignments (#6813). - Since it could only check indent patterns on nodes, it couldn't check the indentation of comments (#3845, #6571) or optional tokens such as parentheses around an expression (#7522) This commit rewrites the `indent` rule. The new strategy is based on tokens rather than nodes: 1. Create a hashmap (`desiredOffsets`). The keys are all the tokens and comments in the file, and the values are objects containing information for a specific offset, measured in indent levels, from a either a specific token or the first column. For example, an element in an array will have `{offset: 1, from: openingCurly}` to indicate that it is offset by one indentation level from the opening curly brace. All the offsets are initialized to 0 at the start. 1. As the AST is traversed, modify the offsets of tokens accordingly. For example, when entering a `BlockStatement`, offset all of the tokens in the `BlockStatement` by 1 from the opening curly brace of the `BlockStatement`. 1. After traversing the AST, calculate the expected indentation levels of every token in the file (according to the `desiredOffsets` map). 1. For each token, compare the expected indentation to the actual indentation in the file, and report the token if the two values are not equal. This has the following advantages: - It is guaranteed to check the indentation of every single token in the file, with the exception of some tokens that are explicitly ignored*. This ensures that no tokens end up unexpectedly being ignored. - Since tokens/comments are used instead of nodes, there are no unchecked "stray tokens". - All nodes are evaluated in a context-free manner. In other words, each node only has to set an offset for its own children, without worrying about what how much indentation the node itself has or what the node's parents are. - The rule ends up with an expected indentation map for the entire file at once, and so it can fix the entire file in one pass. (The previous implementation often required multiple passes. For example, if a node was misaligned with its parent in the previous implementation, the node would get fixed, even if the node's position was actually correct and the parent was off.) *There are a few cases where the new implementation explicitly ignores lines. I decided to do this because there is a huge amount of inconsistency in what people seem to prefer for these cases. If this gets released in a major version, we might want to stop ignoring these cases so that the indentation of all lines is checked. One such case is: ```js ({ foo: bar }); // versus ({ foo: bar }); ``` Comments are treated a bit differently from tokens in that they can have several different indentations. This is because it can be difficult to tell what the comment is referring to. For example: ```js if (foo) { doSomething(); // comment about the doSomething() call } else if (bar.baz()) { doSomethingElse(); } // versus if (foo) { doSomething(); // comment about the bar.baz() call } else if (bar.baz()) { doSomethingElse(); } ``` Specifically, a comment is allowed to have one of three indentations: 1. The same indentation as the token right before it 1. The same indentation as the token right after it 1. The computed indentation for the comment itself --- As mentioned above, the new implementation checks a lot of cases that the old implementation did not check. Based on past experience with fixing bugs in the `indent` rule, I anticipate that this will break quite a lot of builds. We might want to consider holding this off until we release a major version.
not-an-aardvark
added a commit
that referenced
this issue
Feb 18, 2017
Fixes #1801, fixes #3737, fixes #3845, fixes #6007, fixes #6571, fixes #6670, fixes #6813, fixes #7242, fixes #7274, fixes #7320, fixes #7420, fixes #7522, fixes #7616) The existing implementation of `indent` had a lot of bugs (see above list). It worked by detecting a node type (e.g. `ObjectExpression`), and then ensuring that the indentation around the object satisfies certain constraints (e.g. the properties of the `ObjectExpression` are offset by 4 spaces from the opening bracket). This approach had a number of disadvantages: - Since it only checked indentation according to an explicit list of patterns, there were a lot of cases where it accidentally didn't check the indentation at all. For example, there was no check for the indentation of a closing `)` in a `CallExpression`, so the rule just silently ignored incorrect indentation in these cases. (#7522) - there were a lot of nodes where indentation wasn't checked at all. For example, it didn't check indentation for ternary expressions (#7420) or destructuring assignments (#6813). - Since it could only check indent patterns on nodes, it couldn't check the indentation of comments (#3845, #6571) or optional tokens such as parentheses around an expression (#7522) This commit rewrites the `indent` rule. The new strategy is based on tokens rather than nodes: 1. Create a hashmap (`desiredOffsets`). The keys are all the tokens and comments in the file, and the values are objects containing information for a specific offset, measured in indent levels, from a either a specific token or the first column. For example, an element in an array will have `{offset: 1, from: openingCurly}` to indicate that it is offset by one indentation level from the opening curly brace. All the offsets are initialized to 0 at the start. 1. As the AST is traversed, modify the offsets of tokens accordingly. For example, when entering a `BlockStatement`, offset all of the tokens in the `BlockStatement` by 1 from the opening curly brace of the `BlockStatement`. 1. After traversing the AST, calculate the expected indentation levels of every token in the file (according to the `desiredOffsets` map). 1. For each token, compare the expected indentation to the actual indentation in the file, and report the token if the two values are not equal. This has the following advantages: - It is guaranteed to check the indentation of every single token in the file, with the exception of some tokens that are explicitly ignored*. This ensures that no tokens end up unexpectedly being ignored. - Since tokens/comments are used instead of nodes, there are no unchecked "stray tokens". - All nodes are evaluated in a context-free manner. In other words, each node only has to set an offset for its own children, without worrying about what how much indentation the node itself has or what the node's parents are. - The rule ends up with an expected indentation map for the entire file at once, and so it can fix the entire file in one pass. (The previous implementation often required multiple passes. For example, if a node was misaligned with its parent in the previous implementation, the node would get fixed, even if the node's position was actually correct and the parent was off.) *There are a few cases where the new implementation explicitly ignores lines. I decided to do this because there is a huge amount of inconsistency in what people seem to prefer for these cases. If this gets released in a major version, we might want to stop ignoring these cases so that the indentation of all lines is checked. One such case is: ```js ({ foo: bar }); // versus ({ foo: bar }); ``` Comments are treated a bit differently from tokens in that they can have several different indentations. This is because it can be difficult to tell what the comment is referring to. For example: ```js if (foo) { doSomething(); // comment about the doSomething() call } else if (bar.baz()) { doSomethingElse(); } // versus if (foo) { doSomething(); // comment about the bar.baz() call } else if (bar.baz()) { doSomethingElse(); } ``` Specifically, a comment is allowed to have one of three indentations: 1. The same indentation as the token right before it 1. The same indentation as the token right after it 1. The computed indentation for the comment itself --- As mentioned above, the new implementation checks a lot of cases that the old implementation did not check. Based on past experience with fixing bugs in the `indent` rule, I anticipate that this will break quite a lot of builds. We might want to consider holding this off until we release a major version.
not-an-aardvark
added a commit
that referenced
this issue
Feb 23, 2017
Fixes #1801, fixes #3737, fixes #3845, fixes #6007, fixes #6571, fixes #6670, fixes #6813, fixes #7242, fixes #7274, fixes #7320, fixes #7420, fixes #7522, fixes #7616) The existing implementation of `indent` had a lot of bugs (see above list). It worked by detecting a node type (e.g. `ObjectExpression`), and then ensuring that the indentation around the object satisfies certain constraints (e.g. the properties of the `ObjectExpression` are offset by 4 spaces from the opening bracket). This approach had a number of disadvantages: - Since it only checked indentation according to an explicit list of patterns, there were a lot of cases where it accidentally didn't check the indentation at all. For example, there was no check for the indentation of a closing `)` in a `CallExpression`, so the rule just silently ignored incorrect indentation in these cases. (#7522) - there were a lot of nodes where indentation wasn't checked at all. For example, it didn't check indentation for ternary expressions (#7420) or destructuring assignments (#6813). - Since it could only check indent patterns on nodes, it couldn't check the indentation of comments (#3845, #6571) or optional tokens such as parentheses around an expression (#7522) This commit rewrites the `indent` rule. The new strategy is based on tokens rather than nodes: 1. Create a hashmap (`desiredOffsets`). The keys are all the tokens and comments in the file, and the values are objects containing information for a specific offset, measured in indent levels, from a either a specific token or the first column. For example, an element in an array will have `{offset: 1, from: openingCurly}` to indicate that it is offset by one indentation level from the opening curly brace. All the offsets are initialized to 0 at the start. 1. As the AST is traversed, modify the offsets of tokens accordingly. For example, when entering a `BlockStatement`, offset all of the tokens in the `BlockStatement` by 1 from the opening curly brace of the `BlockStatement`. 1. After traversing the AST, calculate the expected indentation levels of every token in the file (according to the `desiredOffsets` map). 1. For each token, compare the expected indentation to the actual indentation in the file, and report the token if the two values are not equal. This has the following advantages: - It is guaranteed to check the indentation of every single token in the file, with the exception of some tokens that are explicitly ignored*. This ensures that no tokens end up unexpectedly being ignored. - Since tokens/comments are used instead of nodes, there are no unchecked "stray tokens". - All nodes are evaluated in a context-free manner. In other words, each node only has to set an offset for its own children, without worrying about what how much indentation the node itself has or what the node's parents are. - The rule ends up with an expected indentation map for the entire file at once, and so it can fix the entire file in one pass. (The previous implementation often required multiple passes. For example, if a node was misaligned with its parent in the previous implementation, the node would get fixed, even if the node's position was actually correct and the parent was off.) *There are a few cases where the new implementation explicitly ignores lines. I decided to do this because there is a huge amount of inconsistency in what people seem to prefer for these cases. If this gets released in a major version, we might want to stop ignoring these cases so that the indentation of all lines is checked. One such case is: ```js ({ foo: bar }); // versus ({ foo: bar }); ``` Comments are treated a bit differently from tokens in that they can have several different indentations. This is because it can be difficult to tell what the comment is referring to. For example: ```js if (foo) { doSomething(); // comment about the doSomething() call } else if (bar.baz()) { doSomethingElse(); } // versus if (foo) { doSomething(); // comment about the bar.baz() call } else if (bar.baz()) { doSomethingElse(); } ``` Specifically, a comment is allowed to have one of three indentations: 1. The same indentation as the token right before it 1. The same indentation as the token right after it 1. The computed indentation for the comment itself --- As mentioned above, the new implementation checks a lot of cases that the old implementation did not check. Based on past experience with fixing bugs in the `indent` rule, I anticipate that this will break quite a lot of builds. We might want to consider holding this off until we release a major version.
eslint-deprecated
bot
added
the
archived due to age
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Per discussion on Gitter Chat, I want to see if Mention Bot can be made more effective by focusing its mention efforts on team members where feasible. This way, folks who made large contributions in the past but who are no longer involved in ESLint receive fewer mentions.
Assumptions/Parameters:
Goals:
I'll be investigating this, but feel free to offer any insight you may already have.
The text was updated successfully, but these errors were encountered: