Issue 10916211: Issue 4752. When convert to getter, remove also ()

Issue 10916211: Issue 4752. When convert to getter, remove also () (Closed)

Created:
8 years, 3 months ago by scheglov

Modified:
8 years, 3 months ago

Reviewers:
Brian Wilkerson

CC:
reviews_dartlang.org

Base URL:
https://dart.googlecode.com/svn/branches/bleeding_edge/dart

Visibility:
Public.

More Reviews

Description

Issue 4752. When convert to getter, remove also () I've forgot to do this initially. Mostly Parser and Core changes. R=brianwilkerson@google.com BUG= Committed: https://code.google.com/p/dart/source/detail?r=12140

Patch Set 2 : Use setter in *Info #

Created: 8 years, 3 months ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+109 lines, -25 lines)			Patch
M	compiler/java/com/google/dart/compiler/ast/DartFunction.java	View		2 chunks	+7 lines, -1 line	0 comments	Download
M	compiler/java/com/google/dart/compiler/parser/DartParser.java	View		7 chunks	+12 lines, -6 lines	0 comments	Download
M	compiler/javatests/com/google/dart/compiler/type/TypeTestCase.java	View		1 chunk	+1 line, -1 line	0 comments	Download
M	editor/tools/plugins/com.google.dart.tools.core/src/com/google/dart/tools/core/dom/AST.java	View		1 chunk	+1 line, -1 line	0 comments	Download
M	editor/tools/plugins/com.google.dart.tools.core/src/com/google/dart/tools/core/internal/completion/ast/FunctionCompleter.java	View		1 chunk	+8 lines, -5 lines	0 comments	Download
M	editor/tools/plugins/com.google.dart.tools.core/src/com/google/dart/tools/core/internal/model/CompilationUnitImpl.java	View	1	3 chunks	+3 lines, -0 lines	0 comments	Download
M	editor/tools/plugins/com.google.dart.tools.core/src/com/google/dart/tools/core/internal/model/DartFunctionImpl.java	View		1 chunk	+6 lines, -0 lines	0 comments	Download
M	editor/tools/plugins/com.google.dart.tools.core/src/com/google/dart/tools/core/internal/model/DartMethodImpl.java	View		1 chunk	+5 lines, -0 lines	0 comments	Download
M	editor/tools/plugins/com.google.dart.tools.core/src/com/google/dart/tools/core/internal/model/info/DartFunctionInfo.java	View	1	2 chunks	+12 lines, -0 lines	0 comments	Download
M	editor/tools/plugins/com.google.dart.tools.core/src/com/google/dart/tools/core/internal/model/info/DartMethodInfo.java	View	1	3 chunks	+12 lines, -0 lines	0 comments	Download
M	editor/tools/plugins/com.google.dart.tools.core/src/com/google/dart/tools/core/model/DartFunction.java	View		1 chunk	+5 lines, -0 lines	0 comments	Download
M	editor/tools/plugins/com.google.dart.tools.ui/src/com/google/dart/tools/internal/corext/SourceRangeFactory.java	View		2 chunks	+10 lines, -0 lines	0 comments	Download
M	editor/tools/plugins/com.google.dart.tools.ui/src/com/google/dart/tools/internal/corext/refactoring/code/ConvertMethodToGetterRefactoring.java	View		4 chunks	+20 lines, -4 lines	0 comments	Download
M	editor/tools/plugins/com.google.dart.tools.ui_test/src/com/google/dart/tools/ui/refactoring/ConvertMethodToGetterRefactoringTest.java	View		3 chunks	+7 lines, -7 lines	0 comments	Download

Messages

Total messages: 3 (0 generated)

Expand Messages | Collapse Messages

Brian Wilkerson

LGTM once my comments have been addressed https://chromiumcodereview.appspot.com/10916211/diff/1/editor/tools/plugins/com.google.dart.tools.core/src/com/google/dart/tools/core/internal/model/info/DartFunctionInfo.java File editor/tools/plugins/com.google.dart.tools.core/src/com/google/dart/tools/core/internal/model/info/DartFunctionInfo.java (right): https://chromiumcodereview.appspot.com/10916211/diff/1/editor/tools/plugins/com.google.dart.tools.core/src/com/google/dart/tools/core/internal/model/info/DartFunctionInfo.java#newcode34 editor/tools/plugins/com.google.dart.tools.core/src/com/google/dart/tools/core/internal/model/info/DartFunctionInfo.java:34: public DartFunctionInfo(int ...

8 years, 3 months ago (2012-09-10 18:01:24 UTC) #2

scheglov

8 years, 3 months ago (2012-09-10 18:27:22 UTC) #3

https://chromiumcodereview.appspot.com/10916211/diff/1/editor/tools/plugins/c...
File
editor/tools/plugins/com.google.dart.tools.core/src/com/google/dart/tools/core/internal/model/info/DartFunctionInfo.java
(right):

https://chromiumcodereview.appspot.com/10916211/diff/1/editor/tools/plugins/c...
editor/tools/plugins/com.google.dart.tools.core/src/com/google/dart/tools/core/internal/model/info/DartFunctionInfo.java:34:
public DartFunctionInfo(int parametersCloseParen) {
On 2012/09/10 18:01:24, Brian Wilkerson wrote:
> The convention followed everywhere else for infos is that the constructor does
> not take any parameters and we define a setter for the value. This is because
of
> the way buildStructure works, which admittedly isn't an issue in this case.
> Nevertheless, I'd like to maintain the convention unless there's a compelling
> reason for changing it.

Done.

https://chromiumcodereview.appspot.com/10916211/diff/1/editor/tools/plugins/c...
File
editor/tools/plugins/com.google.dart.tools.core/src/com/google/dart/tools/core/internal/model/info/DartMethodInfo.java
(right):

https://chromiumcodereview.appspot.com/10916211/diff/1/editor/tools/plugins/c...
editor/tools/plugins/com.google.dart.tools.core/src/com/google/dart/tools/core/internal/model/info/DartMethodInfo.java:38:
public DartMethodInfo(int parametersCloseParen) {
On 2012/09/10 18:01:24, Brian Wilkerson wrote:
> Likewise.

Done.

Expand Messages | Collapse Messages