Page MenuHomePhabricator

[Task] Keyword 'use' inconsistency for php namespaces
Closed, ResolvedPublic

Description

We are sometimes using new \MediawikiCoreClass() instead of adding a use clause. There should be no absolute class references with full namespace in our code. Add a PHPCodeSniffer rule to check for this and fix the remaining cases.

Event Timeline

bzimport raised the priority of this task from to Low.Nov 22 2014, 12:58 AM
bzimport set Reference to bz43295.
bzimport added a subscriber: Unknown Object (MLST).
aude created this task.Dec 20 2012, 12:51 PM

Jeroen: Can you comment on this please?

Lydia_Pintscher removed a subscriber: Unknown Object (MLST).
Lydia_Pintscher removed a subscriber: Unknown Object (MLST).
Krinkle set Security to None.

I think we are moving towards use-ing them, but I'm not sure. @thiemowmde?

Is this about use keywords or @uses tags in tests? The first doesn't need discussion, in my opinion. The second does.

I'm preferring new \TestUser() (to give an example) over use TestUser; in very, very few cases. For a single reason: PHPStorm removes some use lines when pressing Ctrl+Alt+O. But I don't mind (any more) if we are changing all of these to use use.

In general there should not be any \ in the code. I do have the impression that we all agree on that. Therefor I don't think this is an issue that needs to be tracked in Phabricator. I suggest to close this.

Yes we agree. Maybe add a PHPCS rule, as these are added sometimes?

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptSep 10 2015, 4:52 PM
JanZerebecki moved this task from incoming to ready to go on the Wikidata board.Sep 10 2015, 4:52 PM
JanZerebecki added a project: patch-welcome.
JanZerebecki renamed this task from Keyword 'use' inconsistency for php namespaces to [Task] Keyword 'use' inconsistency for php namespaces.Sep 10 2015, 4:57 PM
JanZerebecki updated the task description. (Show Details)

@Lydia_Pintscher I noticed your question, two years late, but oh well :)

In general I think we are doing fine CS wise. I agree with Thiemo that there are some cases where using the FQN is fine or even preferred. Given that, I'm not sure a sniff will be more helpful than annoying. However I'm not against trying it out and seeing what happens.

I think this should just be closed now!

Change 256938 had a related patch set uploaded (by Thiemo Mättig (WMDE)):
Prefer "use" over full qualified class names

https://gerrit.wikimedia.org/r/256938

aude closed this task as Resolved.Dec 4 2015, 3:53 PM
aude assigned this task to thiemowmde.
aude removed a project: Patch-For-Review.

Change 256938 merged by jenkins-bot:
Prefer "use" over full qualified class names

https://gerrit.wikimedia.org/r/256938